Skip to content

Remove build-depends from pkgDescrFieldDescrs #2072

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

Merged
merged 1 commit into from
Nov 3, 2014

Conversation

ezyang
Copy link
Contributor

@ezyang ezyang commented Aug 28, 2014

Remove the build-depends field from the top-level parser of Cabal files.

Why do I think this is OK? Two reasons:

  1. We never consult buildDepends until after Distribution/PackageDescription/Configuration.hs runs, in which case the contents of the field get overwritten with the true dependency picks. We do consult the field for some package syntax checks but as I argue in Cabal.Distribution.PackageDescription.Check should be factored per component #2065 this the wrong place to try to do the check.
  2. The build-depends field already gets lifted up by special case code in sectionizeFields to propagate it to the stanzas, which is the true codepath how we account for these build-depends.

Duncan would feel better if we ran this modification on Hackage to make sure it's good. But based on code reading and light testing, I think it's right.

Signed-off-by: Edward Z. Yang [email protected]

@23Skidoo
Copy link
Member

/cc @dcoutts

@23Skidoo
Copy link
Member

23Skidoo commented Oct 1, 2014

It looks like that there are around 935 (mostly old) packages on Hackage that use this form.

So instead of just removing this field I think we should deprecate it. Top-level build-depends should trigger a warning if Cabal-version is less than 1.21 and an error otherwise.

@ezyang
Copy link
Contributor Author

ezyang commented Nov 3, 2014

@23Skidoo Nah, that's misunderstanding what the patch does. My point is that the top-level parser for build-depends is actually DEAD code: the 935 old packages which use this form, this case is handled with sectionizeFields. So, the expected behavior if you tried to compile one of these with my patched Cabal is that they would work, and indeed, this is the case:

[ezyang@hs01 TrieMap-0.0.1.2]$ ghc --make Setup.lhs -package-db=../.cabal-sandbox/x86_64-linux-
ghc-7.8.3-packages.conf.d/                                                                     
[1 of 1] Compiling Main             ( Setup.lhs, Setup.o )
Linking Setup ...
[ezyang@hs01 TrieMap-0.0.1.2]$ ./Setup configure
Configuring TrieMap-0.0.1.2...
Setup: At least the following dependencies are missing:
containers ==0.2.0.1

@ezyang ezyang force-pushed the ezyang-remove-build-depends branch from eb49c35 to be26378 Compare November 3, 2014 09:35
@23Skidoo
Copy link
Member

23Skidoo commented Nov 3, 2014

Thanks for the explanation. Merging.

23Skidoo added a commit that referenced this pull request Nov 3, 2014
Remove build-depends from pkgDescrFieldDescrs
@23Skidoo 23Skidoo merged commit 7207e68 into haskell:master Nov 3, 2014
@ezyang ezyang deleted the ezyang-remove-build-depends branch April 7, 2016 22:40
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.

2 participants