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

File watch watches unrelated libs #5310

Closed
dten opened this issue Jun 4, 2020 · 9 comments · Fixed by #5317
Closed

File watch watches unrelated libs #5310

dten opened this issue Jun 4, 2020 · 9 comments · Fixed by #5317

Comments

@dten
Copy link
Contributor

dten commented Jun 4, 2020

I have for example a stack.yaml with two packages in, neither of which reference each other. Let's call them package0 and package1

stack build package0 --file-watch

this results in watching all the files of package0 (good) but also all the lib files of package1 (seems bad).

I can't figure out a reason to watch the .hs files and the .md files of the other package. my logic for watching the cabal file also seems a bit weak.

Is this intended behaviour? Am I missing some reasoning where package1 could affect package0 if package0's dependency graph doesn't mention it?

and just in case it's just me i'm on ubuntu 18 under wsl (windows 10 1909) with stack 2.1.3

❯ stack --version
Version 2.1.3, Git revision 636e3a759d51127df2b62f90772def126cdf6d1f (7735 commits) x86_64 hpack-0.31.2
@qrilka
Copy link
Contributor

qrilka commented Jun 4, 2020

@dten could you provide an example project reproducing this?

@dten
Copy link
Contributor Author

dten commented Jun 4, 2020

Sure, here's stack new testlibs copy pasted and renamed

testlibs.zip (edit: this one slightly wrong)

stack build testlibs:lib --file-watch and then watched shows

watched
/mnt/c/Users/DavidHewson/Documents/Repositories/testlibs/stack.yaml
/mnt/c/Users/DavidHewson/Documents/Repositories/testlibs/testlibs/ChangeLog.md
/mnt/c/Users/DavidHewson/Documents/Repositories/testlibs/testlibs/README.md
/mnt/c/Users/DavidHewson/Documents/Repositories/testlibs/testlibs/package.yaml
/mnt/c/Users/DavidHewson/Documents/Repositories/testlibs/testlibs/src/Lib.hs
/mnt/c/Users/DavidHewson/Documents/Repositories/testlibs/testlibs/testlibs.cabal
/mnt/c/Users/DavidHewson/Documents/Repositories/testlibs/testlibs2/ChangeLog.md
/mnt/c/Users/DavidHewson/Documents/Repositories/testlibs/testlibs2/README.md
/mnt/c/Users/DavidHewson/Documents/Repositories/testlibs/testlibs2/package.yaml
/mnt/c/Users/DavidHewson/Documents/Repositories/testlibs/testlibs2/src/Lib.hs
/mnt/c/Users/DavidHewson/Documents/Repositories/testlibs/testlibs2/testlibs2.cabal

edit: hmm i actually made testlib2's executable depend on testlib, so it's not quite what i said

@dten
Copy link
Contributor Author

dten commented Jun 4, 2020

testlibs.zip

fixed not renaming enough in testlibs2. now truly unrelated

@qrilka
Copy link
Contributor

qrilka commented Jun 4, 2020

Thanks, hopefully I will find some time to look into it soon.

@qrilka
Copy link
Contributor

qrilka commented Jun 6, 2020

@dten it looks like currently all local files get tracked not only project package files but also local deps as well see https://github.com/commercialhaskell/stack/blob/master/src/Stack/Build.hs#L67

So it appears that what you want is not the way Stack currently works but changing that seems to be doable and I don't see any arguments against this. @snoyberg what do you think if --file-watch will respect build targets?

@snoyberg
Copy link
Contributor

snoyberg commented Jun 7, 2020

Seems reasonable to me, no objections

@qrilka
Copy link
Contributor

qrilka commented Jun 11, 2020

@snoyberg the fix is quite easy but I'm not sure what do we do with backwards-compatibility. The least surprising would be to add an extra explicit flag, something like --granular-watch without which file watching will work as before. At the same time this makes interface of granular watching somewhat ugly, it would be better to use stack build some-lib --file-watch instead of stack build some-lib --file-watch --granular-watch.
What would you prefer here?

@snoyberg
Copy link
Contributor

I think we can just make the change, perhaps with a --watch-all flag as well to get back current behavior.

@qrilka
Copy link
Contributor

qrilka commented Jun 12, 2020

Thanks, I like this variant

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 a pull request may close this issue.

3 participants