Skip to content

Conversation

@phadej
Copy link
Collaborator

@phadej phadej commented Feb 20, 2016

No description provided.

@phadej phadej force-pushed the package-yaml-modtime-check branch from 1861549 to c6e1fed Compare February 20, 2016 18:16
@mgsloan
Copy link
Contributor

mgsloan commented Feb 21, 2016

Hey, thanks for working on this, #1813 ought to be fixed one way or another.

I don't like this approach because if the user accidentally touches the cabal file, it'll stop updating. That seems very opaque to me, so closing.

@phadej
Copy link
Collaborator Author

phadej commented Feb 21, 2016

@mgsloan but that's why how e.g. make works. User can then touch the package.yaml file? Of course we could save the hash of cabal file contents to be even more sure, but IMHO this is a good first step.

As mentioned, currently stock stack is completely unusable for our company.

@mgsloan
Copy link
Contributor

mgsloan commented Feb 21, 2016

Yeah, I considered this further - since you'd have to touch the *.cabal after the modifying package.yaml, this seems fine.

@mgsloan
Copy link
Contributor

mgsloan commented Feb 21, 2016

@sol What do you think of making this part of hpack's behavior? Or is it not so necessary because you tend to explicitly run it. It surprised me a bit that hpack writing to the file was based on an equality check rather than timestamp check.

@mgsloan
Copy link
Contributor

mgsloan commented Feb 23, 2016

Considering that this also resolves #1828, I think i ought to merge this. Thanks!

One minor reason to do this in hpack is that hpack determines the output cabal file name. This isn't really an issue, though, since we enforce that there's only one cabal file per dir.

mgsloan added a commit that referenced this pull request Feb 23, 2016
Fix #1813: Check package.yaml and cabal file modification times
@mgsloan mgsloan merged commit 17b42de into commercialhaskell:master Feb 23, 2016
@phadej phadej deleted the package-yaml-modtime-check branch February 24, 2016 15:28
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