Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent relative paths with symlinks in scmpuff expand #11

Open
vyder opened this issue Nov 16, 2015 · 8 comments
Open

Inconsistent relative paths with symlinks in scmpuff expand #11

vyder opened this issue Nov 16, 2015 · 8 comments

Comments

@vyder
Copy link

vyder commented Nov 16, 2015

I ran in to an interesting edge case in the way scmpuff_status works - If I'm running the gs from a symlinked repository directory, the filenames are completely expanded out, while git status shows them relative to the actual repository itself.

Example:

With the following folder structure:

~/Code/
    client-name/
        namespace/
            project-repo/
                .git/
                docs
                …
    project-repo-symlink <linked to project-repo>

Difference in behavior between gs and git status:

$ pwd
/User/Vidur/Code
$ cd project-repo-symlink
$ pwd
/User/Vidur/Code/project-repo-symlink
$ git status
On branch master
Untracked files:
  (use "git add <file>..." to include in what will be committed)

    docs/TEST.md
$ gs
# On branch: master  |  [*] => $e*
#
➤ Untracked files
#
#      untracked:  [1] ../client-name/namespace/project-repo/docs/TEST.md
$ git rev-parse --show-toplevel
/User/Vidur/Code/client-name/namespace/project-repo

* 'git' here refers to /usr/local/bin/git, not the scmpuff git wrapper

This seems to be because of the use of git rev-parse --show-toplevel in process.go#105, which identifies the full path to the project root, and I guess the filenames are calculated relative to that.

I'd like to attempt to patch this myself, but I haven't figured out the simplest solution yet. (I also have zero knowledge of Go). Figured I'd make this issue first to track it.

@mroth
Copy link
Owner

mroth commented Nov 16, 2015

We deal with absolute paths internally, see status/process.go and the corresponding tests for the nitty gritty details.

I suspect the issue here actually relates to the expand command, which converts into a relative path relative to the current working directory before output, and in particular convertToRelativeIfFilePath():

// For a given arg, try to determine if it represents a file, and if so, convert
// it to a relative filepath.
//
// Otherwise (or if any error conditions occur) return it unmolested.
func convertToRelativeIfFilePath(arg string) string {
if _, err := os.Stat(arg); err == nil {
wd, err1 := os.Getwd()
relPath, err2 := filepath.Rel(wd, arg)
if err1 == nil && err2 == nil {
return relPath
}
}
return arg
}

My assumption is os.Getwd() on line 96 in the above sample returns the working directory normalized to absolute path. The Go documentation asserts:

If the current directory can be reached via multiple paths (due to symbolic links), Getwd may return any one of them.

So to resolve this we'd likely have to replace it's logic with something custom.

(If you want to start looking into this, as a first step, I'd recommend a failing Aruba integration scenario in features/command_expand.feature so we can test the actual behavior here.)

@mroth mroth changed the title Support for symlinked repositories Inconsistent relative paths with symlinks in scmpuff expand Nov 16, 2015
@mroth mroth added the bug label Nov 16, 2015
@vyder
Copy link
Author

vyder commented Nov 18, 2015

Cool, thanks for the info. That does actually make a lot of sense. I'll give it a shot this weekend and set up a PR.

@mroth mroth added the upstream label Feb 27, 2016
@parkan
Copy link

parkan commented Oct 13, 2016

hi @mroth, I am seeing this behavior with my symlinked golang repos, did a fix ever get committed somewhere? happy to look but don't want to replicate work

@mroth
Copy link
Owner

mroth commented Oct 13, 2016

@parkan a workaround was never added into scmpuff directly, although it's possible Golang may have fixed this upstream in os.Getwd since the most recent version of scmpuff binary was compiled.

@parkan
Copy link

parkan commented Oct 13, 2016

Hmm still seeing this behavior rebuilt against golang 1.7.1, though I actually don't know enough about go dependency mgmt to know if this pulled in the latest os

@hugovk
Copy link

hugovk commented Jun 7, 2018

@vyder Hi, did you ever get round to looking at this?

@tekumara
Copy link

I'm still seeing this when my repo is symlinked, eg:

❯ pwd
/Users/tekumara/store
❯ readlink -f `pwd`
/Users/tekumara/Library/CloudStorage/GoogleDrive/My Drive/store

Using scmpuff 0.5.0:

❯ gs
# On branch: main  |  +2  |  [*] => $e*
#
➤ Changes not staged for commit
#
#        deleted:  [1] ../Library/CloudStorage/GoogleDrive/My Drive/store/food.txt
#

This shortcut can't be used, eg:

❯ gco
fatal: ../Library/CloudStorage/GoogleDrive/My Drive/store/food.txt: '../Library/CloudStorage/GoogleDrive/My Drive/store/food.txt' is outside repository at '/Users/tekumara/Library/CloudStorage/My Drive/store/food.txt'

@tekumara
Copy link

This also manifests when using git reset, numeric shortcuts, and files within a symlinked parent directory, eg:

❯ echo $(pwd)
/home/compute/code/flyte-demo
❯ readlink -f $(pwd)
/mnt/fsx/tekumara/code/flyte-demo
❯ gs
# On branch: main  |  [*] => $e*
#
➤ Changes to be committed
#
#       modified:  [1] ../../../../mnt/fsx/tekumara/code/flyte-demo/Makefile
#       modified:  [2] ../../../../mnt/fsx/tekumara/code/flyte-demo/infra/config.yaml
#
❯ git reset 1
fatal: ambiguous argument '../../../../mnt/tekumara/code/flyte-demo/Makefile': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants