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

Avoid MAX_PATH for precompiled cache #3649 #3702

Merged
merged 1 commit into from
Dec 27, 2017

Conversation

mgsloan
Copy link
Contributor

@mgsloan mgsloan commented Dec 23, 2017

Regression was in 9991156

I have not tested this change, since I do not currently have easy access to windows.

Please include the following checklist in your PR:

  • Any changes that could be relevant to users have been recorded in the ChangeLog.md
  • The documentation has been updated, if necessary.

@mgsloan mgsloan requested a review from snoyberg December 23, 2017 14:50
@borsboom
Copy link
Contributor

Consider rebasing this to the stable branch so it's included in a future minor release (if any).

@mgsloan mgsloan force-pushed the avoid-maxpath-for-precompiled-cache-3649 branch 2 times, most recently from 0487b61 to 880efce Compare December 23, 2017 15:11
@mgsloan mgsloan changed the base branch from master to stable December 23, 2017 15:11
@mgsloan
Copy link
Contributor Author

mgsloan commented Dec 23, 2017

Good point! I have rebased and retargetted stable

@mgsloan mgsloan mentioned this pull request Dec 23, 2017
@mgsloan mgsloan force-pushed the avoid-maxpath-for-precompiled-cache-3649 branch from 880efce to 2a69f1e Compare December 25, 2017 17:23
@mgsloan mgsloan force-pushed the avoid-maxpath-for-precompiled-cache-3649 branch from 2a69f1e to 6e6934d Compare December 25, 2017 17:26
@mgsloan mgsloan merged commit bf31549 into stable Dec 27, 2017
@mungre
Copy link

mungre commented Dec 28, 2017

I tried this new version on Windows and I'm still getting what appear to be path length errors building projects.

An error occurs after building a small number of dependencies, usually less than ten. The weird thing is that when I restart the build it continues as if nothing had gone wrong, and then fails again after building a few more dependencies. If the build is repeatedly restarted then it will eventually finish.

I deleted stack's cache (C:\users\mungre\AppData\Roaming\stack) before building my project.

This is the output from a couple of turns around this loop:

>stack --version
Version 1.6.4, Git revision bf31549797845d38c980703d00e1eb247338a16f (5466 commits) x86_64 hpack-0.20.0

>stack build
basement-0.0.4: download
byteable-0.1.1: download
byteable-0.1.1: configure
byteable-0.1.1: build
basement-0.0.4: configure
byteable-0.1.1: copy/register
byteorder-1.0.4: download
basement-0.0.4: build
byteorder-1.0.4: configure
byteorder-1.0.4: build
byteorder-1.0.4: copy/register
bytestring-builder-0.10.8.1.0: download
bytestring-builder-0.10.8.1.0: configure
bytestring-builder-0.10.8.1.0: build
bytestring-builder-0.10.8.1.0: copy/register
basement-0.0.4: copy/register
Progress: 4/177C:\Users\mungre\AppData\Roaming\stack\precompiled\x86_64-windows\ghc-8.2.2\2.0.1.0\Ynl0ZXN0cmluZy1idWlsZGVyLTAuMTAuOC4xLjBAc2hhMjU2OjY5ZDQ0ZWNmY2YyNDMzOTBmNTgyYTUxNTkwNWQ2MTRiMGRjNWMzMThlYWQ2MjFjYTU2NDFjZmJkMGFhNTZkZmYsMzc0Ng==\1f1C--p9_VorahgpH19QXy0W5mnh-O-BAhKtT-cKew4=: openBinaryFile: does not exist (No such file or directory)

>stack build
cabal-doctest-1.0.4: download
cereal-0.5.4.0: download
cabal-doctest-1.0.4: configure
cabal-doctest-1.0.4: build
cereal-0.5.4.0: configure
cereal-0.5.4.0: build
cabal-doctest-1.0.4: copy/register
data-default-class-0.1.2.0: download
data-default-class-0.1.2.0: configure
data-default-class-0.1.2.0: build
data-default-class-0.1.2.0: copy/register
cereal-0.5.4.0: copy/register
Progress: 3/173C:\Users\mungre\AppData\Roaming\stack\precompiled\x86_64-windows\ghc-8.2.2\2.0.1.0\ZGF0YS1kZWZhdWx0LWNsYXNzLTAuMS4yLjBAc2hhMjU2OjYzZTYyMTIwYjdlZmQ3MzNhNWExN2NmNTljZWI0MzI2OGU5YTkyOWM3NDgxMjcxNzJkN2Q0MmY0YTMzNmUzMjcsNTQy\wVWqBSSq0fCfcwGy8J2YefUrKJnfifuo0K3EVuDHpuc=: openBinaryFile: does not exist (No such file or directory)

@mgsloan
Copy link
Contributor Author

mgsloan commented Dec 29, 2017

@mungre Curious! Could you please add some debug logging? Maybe something like logInfo $ T.pack (show (toFilePath longPath)) <> " with length: " <> T.pack (show (length (toFilePath longPath))). Maybe there's some other codepath that uses these paths.

@mungre
Copy link

mungre commented Dec 29, 2017

@mgsloan I extended the logging to show both the original path and the shortened path; they were always identical. So I added logging to the case expression that checks the path length. It always took the maxPathLength=Nothing branch.

Of course, the error lies in the one thing the compiler doesn't validate!

diff --git a/src/Stack/Constants.hs b/src/Stack/Constants.hs
index 81298df..75b2db4 100644
--- a/src/Stack/Constants.hs
+++ b/src/Stack/Constants.hs
@@ -247,7 +247,7 @@ defaultTerminalWidth = 100
 -- | Maximum length to use in paths. Is only a 'Just' value on windows,
 -- corresponding to MAX_PATH.
 maxPathLength :: Maybe Int
-#ifdef mings32_HOST_OS
+#ifdef mingw32_HOST_OS
 maxPathLength = Just 260
 #else
 maxPathLength = Nothing

It seems to have been able to resume failed builds because the files it is creating are just pointers to other files (as far as I can see) and they don't actually seem to be required.

mgsloan added a commit that referenced this pull request Dec 29, 2017
@mgsloan
Copy link
Contributor Author

mgsloan commented Dec 29, 2017

@mungre LOL, my mistake on the typo. Thanks for figuring it out! Fixed in 36c1226

@mungre
Copy link

mungre commented Dec 31, 2017

@mgsloan Thanks for fixing this.

I've just had another silly failure: it doesn't work when the path length is exactly 260 characters. Microsoft's MAX_PATH includes room for a terminating null character, so the maximum path is actually 259 characters. So the "Just 260" should be "Just 259".

I think this could still fail with astral plane characters (Haskell counts them as one but Windows counts them as two) but they are unlikely in this context anyway.

@mgsloan
Copy link
Contributor Author

mgsloan commented Dec 31, 2017

@mungre Good catch! I was a little worried about corner cases with the number. Feel like opening a PR adjusting that?

Would be great to fix the astral plane char thing too, shouldn't be too hard, just a fold over Text

@mungre
Copy link

mungre commented Dec 31, 2017

@mgsloan Yes, I'll have a go.

mungre pushed a commit to mungre/stack that referenced this pull request Jan 1, 2018
@mgsloan mgsloan deleted the avoid-maxpath-for-precompiled-cache-3649 branch January 2, 2018 09:52
mgsloan added a commit that referenced this pull request Jan 11, 2018
Fix off-by-one and char encoding issue #3649 #3702
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants