(hopeful) fix for #2

[cspotcode]

trafficstars

I have only informally tested this locally, so I can't prove it works, and I haven't had time to write tests.

Idea is pretty simple: use [[ -L to check if final path is to a symlink. Use readlink to read it, concatenate the path back together, and try the entire thing all over again using the newly computed path.

#2

[cspotcode]

I added a test, and it looks like it's working.

[morgant]

Thanks for submitting this fix, I'm in the process of testing it.

[cspotcode]

I just realized your README says you explicitly avoid using readlink and this PR uses readlink. However, I think it's ok because we're not using any special flags so it seems compatible with the readlink bundled in MacOS.

[morgant]

I had completely forgotten about not wanting a readlink prerequisite. It certainly appears to be available under macOS 10.14 Mojave, but must not have been included in prior versions. I'll look into that, as well.

[morgant]

I believe that pwd -P might allow us to work around the readlink requirement.

[cspotcode]

That will only work for directories, not files, right? If I have a symlink file at /foo/bar/baz.txt that points to /something/totally/different.txt, and I want pwd -P to tell me where it points, I need to set my working directory to /foo/bar/baz.txt.

Is that possible, since baz.txt is not a directory?

On Sun, Sep 15, 2019, 1:46 PM Morgan Aldridge [email protected] wrote:

I believe that pwd -P might allow us to work around the readlink requirement.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/morgant/realpath/pull/3?email_source=notifications&email_token=AAC35OEKKPFXTYRFTBVGJELQJZYIXA5CNFSM4IUK55MKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6XVRYI#issuecomment-531585249, or mute the thread https://github.com/notifications/unsubscribe-auth/AAC35OG4T5YV4ESKSHB2TUTQJZYIXANCNFSM4IUK55MA .

[morgant]

@cspotcode Good point, pwd -P won't work when the file itself is a symlink. I've added tests for files being symlinks (which is broken) and paths including symlinks (which work).

I think that readlink is available on most platforms, but some of the options (-e?) which are often used to implement the whole of realpath are not. That said, I think we should be able to use readlink to resolve a symlink. I'll review your code again.

[morgant]

Honestly, I'm not fully a fan of the while/continue, but it works and is pretty minimal. It passes the new tests, which is good.

If you want to merge the updated tests into your branch I'll merge this. Huge thanks for catching this glaring bug and fixing it!

[cspotcode]

Thanks for taking a look! The while-continue was laziness on my part; it was the first implementation I thought of.

I'm on vacation till Monday but can merge Monday evening.

On Sat, Sep 28, 2019, 2:01 PM Morgan Aldridge [email protected] wrote:

Honestly, I'm not fully a fan of the while/continue, but it works and is pretty minimal. It passes the new tests, which is good.

If you want to merge the updated tests into your branch I'll merge this. Huge thanks for catching this glaring bug and fixing it!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/morgant/realpath/pull/3?email_source=notifications&email_token=AAC35OAS4CMAXOSBLV2SQJTQL6LYRA5CNFSM4IUK55MKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD727BOI#issuecomment-536211641, or mute the thread https://github.com/notifications/unsubscribe-auth/AAC35OFR34GXID3S4HU6C2LQL6LYRANCNFSM4IUK55MA .