Skip to content
This repository has been archived by the owner on Aug 2, 2020. It is now read-only.

Shake fails to rebuild recover from deleted libraries/integer-gmp/gmp/config.mk #433

Open
bgamari opened this issue Oct 11, 2017 · 8 comments
Assignees

Comments

@bgamari
Copy link
Collaborator

bgamari commented Oct 11, 2017

While testing my fix for #432 I tried deleting libraries/integer-gmp/gmp/config.mk to force Hadrian to reconfigure gmp. However, instead of rerunning configure Shake fails with a perplexing error,

shakeArgsWith   0.000s    0%
Function shake  0.182s    1%
Database read   0.063s    0%
With database   0.084s    0%
Running rules   9.011s   96%  =========================
Total           9.340s   99%
Error when running Shake build system:
* inplace/lib/package.conf.d/ghc-8.3.conf
* _build/stage1/compiler/inplace-pkg-config
* _build/stage1/compiler/package-data.mk
* _build/stage1/compiler/package-data.mk _build/stage1/compiler/setup-config
* _build/stage1/gmp/include/ghc-gmp.h
* libraries/integer-gmp/gmp/config.mk
* libraries/integer-gmp/gmp/config.mk libraries/integer-gmp/integer-gmp.buildinfo
Error, &%> rule failed to build 1 file (out of 2)
  libraries/integer-gmp/gmp/config.mk - MISSING
  libraries/integer-gmp/integer-gmp.buildinfo
CallStack (from HasCallStack):
  error, called at src/Development/Shake/Internal/Rules/Files.hs:210:13 in shake-0.16-5369d5db762e7ec9555c1f0223da3efe654f2a308099bec4a061a426dc70b106:Development.Shake.Internal.Rules.Files

Indeed the file Shake complains about is missing: I intentionally deleted it. What I don't understand is why shake fails instead of simply rebuilding it. It surely knows how to do so, there is a rule for it in Rules.Gmp, which clearly has realized.

Thinking that perhaps it doesn't like to build multi-target rules where only some targets are out of date, I then tried deleting libraries/integer-gmp/integer-gmp.buildinfo as well. Unfortunately this just resulting in Shake complaining about both files,

Error when running Shake build system:
* inplace/lib/package.conf.d/ghc-8.3.conf
* _build/stage1/compiler/inplace-pkg-config
* _build/stage1/compiler/package-data.mk
* _build/stage1/compiler/package-data.mk _build/stage1/compiler/setup-config
* _build/stage1/gmp/include/ghc-gmp.h
* libraries/integer-gmp/gmp/config.mk
* libraries/integer-gmp/gmp/config.mk libraries/integer-gmp/integer-gmp.buildinfo
Error, &%> rule failed to build 2 files (out of 2)
  libraries/integer-gmp/gmp/config.mk - MISSING
  libraries/integer-gmp/integer-gmp.buildinfo - MISSING
CallStack (from HasCallStack):
  error, called at src/Development/Shake/Internal/Rules/Files.hs:210:13 in shake-0.16-5369d5db762e7ec9555c1f0223da3efe654f2a308099bec4a061a426dc70b106:Development.Shake.Internal.Rules.Files
@ndmitchell
Copy link
Collaborator

The problem is Shake doesn't like you to declare a rule that builds two targets, and after running the rule, they aren't available. My guess is the rule that builds those two doesn't actually build those two files, but depends on something that does build those two files, and which you didn't dirty/delete. The solution would then be to make the rule directly build them.

@snowleopard
Copy link
Owner

snowleopard commented Oct 11, 2017

Right, it's unclear how to easily fix this. Here is the problematic line:

https://github.com/snowleopard/hadrian/blob/master/src/Rules/Gmp.hs#L82

The package-data.mk file is up-to-date, so we do not rerun ghc-cabal configure. And here is the rule that runs ghc-cabal configure:

https://github.com/snowleopard/hadrian/blob/master/src/Rules/Data.hs#L19

As you can see, it's a generic rule, handling all GHC packages, and we do not know upfront what will be the exact set of output files for a given package. We know for sure the rule will generate package-data.mk file, but libraries/integer-gmp/gmp/config.mk is generated only for integer-gmp. The right solution seems to be to create a separate build rule for integer-gmp/package-data.mk. If there are no better ideas, I could implement this (in a few days, as I'm on Haskell eXchange until Saturday).

@bgamari
Copy link
Collaborator Author

bgamari commented Oct 11, 2017

Yes, that makes sense and indeed it looks like that is what is happening here.

The solution would then be to make the rule directly build them.

I'm not sure how to best reorganize the rules such that this property holds. It seems that libraries/integer-gmp/gmp/config.mk is generated by the package-data.mk rule, defined generally in Rules.Data.buildPackageData. Indeed the rule for libraries/integer-gmp/gmp/config.mk depends upon this rule; however, package-data.mk appears to be up-to-date and consequently its actions are never run.

It seems like the solution is to ensure that files generated by autoconf are also counted as targets of the package-data.mk rule. However, it's not clear how to make that so. @snowleopard, any thoughts?

@bgamari
Copy link
Collaborator Author

bgamari commented Oct 11, 2017

Whoops, indeed it look like we came to the same conclusion @snowleopard; your comment didn't show up until I had already submitted.

@bgamari
Copy link
Collaborator Author

bgamari commented Oct 11, 2017

The right solution seems to be to create a separate build rule for integer-gmp/package-data.mk. If there are no better ideas, I could implement this (in a few days, as I'm on Haskell eXchange until Saturday).

That would be great; I'm currently doing what I can to make sure that the various build configurations work as expected in preparation for merging.

@snowleopard
Copy link
Owner

snowleopard commented Oct 11, 2017

@bgamari Aha, we're on the same page :-) Here's what I propose:

  • Factor out the package-data.mk rule into a function and export it from the Rules.Data module.
  • In Rules.Gmp add a higher-priority build rule for integer-gmp's package-data and config.mk, which simply calls the exported build function.

If you're up to implementing this, go ahead without waiting for me, otherwise I'll pick this up later.

@snowleopard snowleopard self-assigned this Oct 17, 2017
@snowleopard
Copy link
Owner

While trying to fix this I got the following Shake error:

All patterns to &%> must have the same number and position of // and * wildcards
* //stage0/libraries/integer-gmp/package-data.mk
* //stage0/libraries/integer-gmp/setup-config
* libraries/integer-gmp/gmp/config.mk (incompatible)

As a workaround, I modified the last pattern to start with the // wildcard as well (yes, that's a hack). But this then failed with the following error:

  _build/stage1/libraries/integer-gmp/package-data.mk
  _build/stage1/libraries/integer-gmp/setup-config
  _build/libraries/integer-gmp/gmp/config.mk - MISSING

The file libraries/integer-gmp/gmp/config.mk was built, but Shake clearly expected all // wildcards to resolve to the same string. @ndmitchell Any suggestions? Note that I don't know the build directory _build statically, that's why I rely on the // pattern in build rules.

@bgamari One possible solution is to move libraries/integer-gmp/gmp/config.mk into _build directory, which is probably the Right Thing to do, but that will require a change to integer-gmp's configure. Perhaps, we can make this conditional (passing a directory for storing generated files, e.g. via a flag).

snowleopard added a commit that referenced this issue Oct 18, 2017
@bgamari
Copy link
Collaborator Author

bgamari commented Oct 19, 2017

@bgamari One possible solution is to move libraries/integer-gmp/gmp/config.mk into _build directory, which is probably the Right Thing to do, but that will require a change to integer-gmp's configure. Perhaps, we can make this conditional (passing a directory for storing generated files, e.g. via a flag).

Yes, this sounds reasonable to me. In principle autoconf supports out-of-tree builds. That is to say, you can run configure in any directory, including one that isn't the source root, and it will generate a build system there. So, for instance, if I do the following starting in the ghc root directory,

$ mkdir tmp
$ cd tmp
$ libraries/integer-gmp/configure

I get a bunch of files in tmp/, including gmp/config.mk.

Unfortunately, I'm not sure it will be easy to convince Cabal to do this. @hvr, @ezyang, is there any way to get Cabal's configure to run configure from an arbitrary directory?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants