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

canonicalizePath "." has changed behaviour #42

Closed
ndmitchell opened this issue Feb 8, 2016 · 6 comments
Closed

canonicalizePath "." has changed behaviour #42

ndmitchell opened this issue Feb 8, 2016 · 6 comments
Assignees
Labels
type: a-bug The described behavior is not working as intended.

Comments

@ndmitchell
Copy link

The the version of directory that ships with GHC 8.0 RC2 I see canonicalizePath "." == "C:\\Neil\\". On GHC 7.10 it's "C:\\Neil" without the trailing slash. This change of behaviour has broken the Shake test suite, and is not documented in the changelog. If it's an intentional change, it should be in the changelog. If it's unintentional, you still have a small window to revert it.

CC @hvr, who seems to be doing GHC 8.0 release management things.

@hvr
Copy link
Member

hvr commented Feb 8, 2016

/cc'ing also @bgamari whom I help with rls mgmt, but who is ultimately the one in charge and spending his weekends with GHC 8 release managament... :-)

@hvr
Copy link
Member

hvr commented Feb 8, 2016

@ndmitchell just to clarify, the semantic change you're complaining occured in the hackage-released v1.2.5 release, and would therefore constitute a PVP violation?

@ndmitchell
Copy link
Author

@hvr - I tried 7.10 which seems to have 1.2.2.0 and it doesn't include the trailing slash. I tried 1.2.5.0 and it does. I'm not really complaining about PVP violations, more undocumented change that might or might not have been intentional. For info, I've fixed this in Shake with ndmitchell/shake@9f6f901.

@Rufflewind
Copy link
Member

The change occurred in 1.2.3.0. It was not intentional.

The new implementation of canonicalizePath makes use of makeAbsolute, which uses normalise to clean up the path. Hence, this is a manifestation of how normalise currently works.

I'm not sure what's the "right" behavior for this, so I'll just revert to the old behavior.

(This change will also affect makeAbsolute.)

@Rufflewind Rufflewind self-assigned this Feb 8, 2016
@Rufflewind Rufflewind added the type: a-bug The described behavior is not working as intended. label Feb 8, 2016
@ndmitchell
Copy link
Author

Specifically, what are you passing into normalise? And does normalise do anything other than make the drive upper case on Windows? If not, I'd be tempted to implement that as a Windows-only single tweak, rather than use normalise, which is a big hammer which is a little dodgy at the best of times.

@Rufflewind
Copy link
Member

It looks roughly like this:

makeAbsolute path = normalise . (</> path) <$> getCurrentDirectory

canonicalizePath path = normalise <$> (transform =<< makeAbsolute path)
  where transform = {- platform-specific function -}

Besides uppercasing the drive letter, normalise also removes redundant dots and slashes, and changes to the correct type of slash (Windows-only).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: a-bug The described behavior is not working as intended.
Projects
None yet
Development

No branches or pull requests

3 participants