Skip to content

ln: Treat parameters consistently with native ln.#235

Closed
Schoonology wants to merge 1 commit intomasterfrom
relative-symlinks
Closed

ln: Treat parameters consistently with native ln.#235
Schoonology wants to merge 1 commit intomasterfrom
relative-symlinks

Conversation

@Schoonology
Copy link
Copy Markdown
Contributor

@Schoonology Schoonology commented Sep 15, 2015

Previously, ln resolved input paths to abspaths, moving the root of relative paths to $PWD, whereas native ln treats the source path as relative to the destination. This restores the latter behaviour.

This closes #100.

Remaining work:

  • Test on Windows
  • Test on Linux

Is anyone available for this testing?

RFC @arturadib @charlierudolph @nylen

@Schoonology
ln: Treat parameters consistently with native ln.
825b80e 
Previously, ln resolved input paths to abspaths, moving the root
of relative paths to $PWD, whereas native `ln` treats the source
path as relative to the _destination_. This restores the latter
behaviour.
@nylen
Copy link
Copy Markdown

nylen commented Sep 17, 2015

Confirming that links are created consistently with /bin/ln on Linux. However, unlike /bin/ln, this warning is still displayed when following my test instructions from #100 (thanks, past self):

ln: Source file does not exist

Fortunately for my sanity I no longer have to use windows and therefore do not care about that even a little bit.

@nylen
Copy link
Copy Markdown

nylen commented Sep 17, 2015

Also the Source file does not exist warning is incorrect in my test case:

If you really want to check for the existence of the source file, resolve the source path relative to the destination directory, but POSIX ln doesn't do this.

@freitagbr
Copy link
Copy Markdown
Contributor

freitagbr commented Jan 10, 2016

Source file does not exist occurs because fs.existsSync(source) checks for source relative to the current directory, and not relative to the destination directory.

@freitagbr freitagbr mentioned this pull request Jan 11, 2016
Merged
@ariporad
Copy link
Copy Markdown
Contributor

ariporad commented Jan 24, 2016

@nfischer: Could you test this on windows?

@ariporad ariporad added feature medium priority bash compat Compatibility issues with bash or POSIX behavior labels Jan 24, 2016
@nfischer
Copy link
Copy Markdown
Member

nfischer commented Jan 24, 2016

@ariporad I think we're favoring #282, which is based off this but fixes the issue @freitagbr points out above. Once that is merged, we should probably close this.

@ariporad
Copy link
Copy Markdown
Contributor

ariporad commented Jan 25, 2016

Ok

@nfischer
Copy link
Copy Markdown
Member

nfischer commented Jan 26, 2016

Closing this, since #282 should fix this

@nfischer nfischer closed this Jan 26, 2016
@nfischer nfischer deleted the relative-symlinks branch July 4, 2018 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

bash compat Compatibility issues with bash or POSIX behavior feature medium priority

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Symbolic links are broken!

5 participants

@Schoonology @nylen @freitagbr @ariporad @nfischer