Skip to content

Conversation

@tswelsh
Copy link
Contributor

@tswelsh tswelsh commented Nov 10, 2017

Attempt at a fix for #3483, following the comment there. I'm not quite up on the internals of stack so let me know if this is the wrong direction. It raised a couple of questions about how things work, which may make it into their own issues. One in particular: is the library component of a package always built, regardless of targets?

Note: Documentation fixes for https://docs.haskellstack.org/en/stable/ should target the "stable" branch, not master.

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.

Please also shortly describe how you tested your change. Bonus points for added tests!

@tswelsh
Copy link
Contributor Author

tswelsh commented Nov 15, 2017

Just realised I had the checklist etc but hadn't actually done it. How I tested: I used a project with a single package with an executable and test suite. Ran stack build --file-watch, edited the test suite Main.hs and observed no rebuild. Edited the executable's main file and it did rebuild.

@snoyberg
Copy link
Contributor

This looks OK, and I've approved, but just doing a little more testing. I noticed some strange behavior, not sure if it's this patch or something else.

Copy link
Contributor

@snoyberg snoyberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I've confirmed that there's something wrong with this patch. When working on the Stack codebase itself: if I use a stack executable built from master, all is well. Using this branch, I see the following:

$ stack build --file-watch
stack-1.6.0: unregistering (local file changes: src/test/Network/HTTP/Download/VerifiedSpec.hs src/test/Spec.hs src/test/Stack/ArgsSpec.hs src/te...)
stack-1.6.0: build (lib + exe)
Preprocessing library for stack-1.6.0..
Building library for stack-1.6.0..
Preprocessing executable 'stack' for stack-1.6.0..
Building executable 'stack' for stack-1.6.0..
stack-1.6.0: copy/register
Installing library in /Users/michael/Documents/stack/.stack-work/install/x86_64-osx/lts-9.14/8.0.2/lib/x86_64-osx-ghc-8.0.2/stack-1.6.0-5MKfzwUAGDM5FSQ3rYxlAo
Installing executable stack in /Users/michael/Documents/stack/.stack-work/install/x86_64-osx/lts-9.14/8.0.2/bin
Registering library for stack-1.6.0..
ExitSuccess
Type help for available commands. Press enter to force a rebuild.

stack-1.6.0: unregistering (local file changes: src/test/Network/HTTP/Download/VerifiedSpec.hs src/test/Spec.hs src/test/Stack/ArgsSpec.hs src/te...)
stack-1.6.0: build (lib + exe)
Preprocessing library for stack-1.6.0..
Building library for stack-1.6.0..
Preprocessing executable 'stack' for stack-1.6.0..
Building executable 'stack' for stack-1.6.0..
stack-1.6.0: copy/register
Installing library in /Users/michael/Documents/stack/.stack-work/install/x86_64-osx/lts-9.14/8.0.2/lib/x86_64-osx-ghc-8.0.2/stack-1.6.0-5MKfzwUAGDM5FSQ3rYxlAo
Installing executable stack in /Users/michael/Documents/stack/.stack-work/install/x86_64-osx/lts-9.14/8.0.2/bin
Registering library for stack-1.6.0..
ExitSuccess
Type help for available commands. Press enter to force a rebuild.

stack-1.6.0: unregistering (local file changes: src/test/Network/HTTP/Download/VerifiedSpec.hs src/test/Spec.hs src/test/Stack/ArgsSpec.hs src/te...)
stack-1.6.0: build (lib + exe)
Preprocessing library for stack-1.6.0..
Building library for stack-1.6.0..
Preprocessing executable 'stack' for stack-1.6.0..
Building executable 'stack' for stack-1.6.0..
stack-1.6.0: copy/register
Installing library in /Users/michael/Documents/stack/.stack-work/install/x86_64-osx/lts-9.14/8.0.2/lib/x86_64-osx-ghc-8.0.2/stack-1.6.0-5MKfzwUAGDM5FSQ3rYxlAo
Installing executable stack in /Users/michael/Documents/stack/.stack-work/install/x86_64-osx/lts-9.14/8.0.2/bin
Registering library for stack-1.6.0..
ExitSuccess
Type help for available commands. Press enter to force a rebuild.

Note that I have made no file changes in the interim.

@mgsloan
Copy link
Contributor

mgsloan commented Nov 26, 2017

Hmm, I'm stumped. Haven't tried to reproduce it, but the change looks good. In particular:

  • This should only affect lpFiles, and it is only used for file watching.

  • Otherwise, it seems just like an inlining of getPackageFilesSimple, along with selection of which components to build.

Curious!

@tswelsh
Copy link
Contributor Author

tswelsh commented Nov 28, 2017

OK, I have reproduced the issue spotted by @snoyberg. I believe the unregistering (local file changes: ... ) is due to a call to checkForUnlistedFiles, called by postBuildCheck, within Stack.Build.Execute.singleBuild. This ultimately uses getPackageFilesSimple which gets all files including those to do with test suites. As the test suite files aren't in the build cache, they get added by this check and appear to be dirty the next time round, even though they aren't being used. Phew!

I've added a commit which uses the same logic in both places, so the ultimate effect of all the changes is more or less to replace getPackageFilesSimple with a new, pickier function getPackageFilesForTargets.

@snoyberg snoyberg merged commit 7af4393 into commercialhaskell:master Nov 28, 2017
@snoyberg
Copy link
Contributor

Looks great now, thanks!

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.

3 participants