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.

Sign up for GitHub

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

Jump to bottom

Inconsistent relative paths with symlinks in scmpuff expand #11

Open
vyder opened this issue Nov 16, 2015 · 8 comments · May be fixed by #85
Open

Inconsistent relative paths with symlinks in scmpuff expand #11

vyder opened this issue Nov 16, 2015 · 8 comments · May be fixed by #85
Labels
bug upstream

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():

scmpuff/commands/expand/expand.go

Lines 90 to 103 in c25f0e3

// 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

@mroth mroth mentioned this issue Oct 16, 2017
Closed
@hugovk
Copy link

hugovk commented Jun 7, 2018

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

@mroth mroth mentioned this issue Dec 13, 2021
Closed
@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>...]'

christoph-heiss added a commit to christoph-heiss/scmpuff that referenced this issue Dec 2, 2024
@christoph-heiss
status: try reading working directory path from $PWD first
37509d7 
Fixes mroth#11.

[0] mroth#11

Signed-off-by: Christoph Heiss <[email protected]>
@christoph-heiss christoph-heiss linked a pull request Dec 2, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug upstream
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

status: try reading working directory path from $PWD first christoph-heiss/scmpuff
5 participants
@mroth @tekumara @parkan @vyder @hugovk