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

Rules for rts/rts.cabal are broken #655

Open
ndmitchell opened this issue Aug 13, 2018 · 15 comments
Open

Rules for rts/rts.cabal are broken #655

ndmitchell opened this issue Aug 13, 2018 · 15 comments

Comments

@ndmitchell
Copy link
Collaborator

Two observations:

  • Changing rts/rts.cabal.in doesn't seem to change rts/rts.cabal.
  • Deleting rts/rts.cabal and then rebuilding results in a complaint that the rule that was meant to produce rts.cabal didn't.

Seems like there's a few issues around there? This is on Mac with the --configure argument not set.

Reason for wanting to tweak rts.cabal.in after the fact was #614.

@snowleopard
Copy link
Owner

To produce rts.cabal from rts.cabal.in we need to run the configure script, but a few people demanded the corresponding rule to be locked behind the --configure flag.

So, this is not a bug but rather an unfortunate and bad (in my opinion) design decision. I'm happy to unlock the configure rule but regularly going back and forth on this is a bit silly. We should finally make this decision for good.

@ndmitchell
Copy link
Collaborator Author

I must say it's hard to distinguish this behaviour from a bug... And I guess people are also required to know when to rerun the configure script, and what inputs it takes? It seems a bit of a nightmare the way it currently is... Anyway, happy to close this as "expected" behaviour.

@snowleopard
Copy link
Owner

snowleopard commented Aug 13, 2018

I agree that this looks and quacks like a bug.

@bgamari @hvr @angerman: I believe you all insisted on not allowing Hadrian to run the configure script automatically. What do you think of this issue?

I think for non-expert users who don't build GHC every day this will definitely be very confusing. On the other hand, expert users who know and use various special flags with the configure script can certainly learn to run Hadrian with the --skip-configure flag if they want to do the configuration step manually.

@Mistuke
Copy link
Contributor

Mistuke commented Aug 16, 2018

I'd just like to throw my hat in saying that I agree with @angerman , @hvr and @bgamari here.

I hate autoconf with a passion, but given that it's there I'd like it to work on the principle of least surprises. Every build system with autoconf I've ever used separates out configuration and building. I would find it very annoying if the build system reconfigured my tree without me asking.

Aside from it being unusual, I'm also not sure how you'd actually get it correct either. Would you be extracting the configuration flags from config.status to know how to re-invoke configure? How will you determine any environment variables I've set when I configured the tree that aren't there anymore.

When configure touches and regenerates other configuration files like ghcautoconf.h, will this trigger hadrian to do a full rebuild since this file touches almost everything?

The amount of ways this could go wrong is quite large, why open up that can of worms. It seems quite logical to me that if you make the conscious effort to change a configuration input file, you must manually reconfigure. You never know the state of my console or environment at the reconfigure time and must not assume it's the same as it originally was.

@ndmitchell
Copy link
Collaborator Author

@Mistuke how do you concretely suggest solving the problem at the top? How am I to know that changing rts/rts.cabal.in requires a configure? Or alternatively, should it? If we focus on the specific problem here, and how to make it better for me, so I don't screw up again :)

@hvr
Copy link
Contributor

hvr commented Aug 16, 2018

@ndmitchell do what I wanted to do for the makefile-based system already but refrained from (because Hadrian came along and made it seem like wasted effort): let the build-system perform the x.in -> x transformation, rather than the ./configure phase; let the ./configure phase dump all key/value pairs we'd be interested to @@-replace into an intermediate key/value mapping text-file, which is then used by the build-system to perform the .in-replacements.

@ndmitchell
Copy link
Collaborator Author

@hvr sounds good! If the configure output was purely an input (had no significant dependencies on in-source stuff) then it's much easier to do either way.

@hvr
Copy link
Contributor

hvr commented Aug 16, 2018

Are there any significant dependencies to in-source entities you're worried about?

@Mistuke
Copy link
Contributor

Mistuke commented Aug 16, 2018

@ndmitchell the typical way you solve this is having the build system compare the timestamps on the input and generated file and issue an appropriate warning.

Now whether you want this to be a warning or a hard abort can be a config flag, I assume Hadrian has an equivalent to build.mk. Where you can tell it to abort if you want.

@ndmitchell
Copy link
Collaborator Author

@hvr rts.cabal.in is the one that's tripped me thus far. The fewer the better!

@Mistuke that seems like threading a complex needle. If we go for what @hvr suggested we eliminate the problem at source.

@snowleopard
Copy link
Owner

I like @hvr's solution. Most of the @@ settings are already available to Hadrian via system.config produced by the configure script, so it's just a matter of adding a rule that does string replacement.

@Mistuke
Copy link
Contributor

Mistuke commented Aug 17, 2018

@hvr solution just moves the goal post, a person would then modify configure and wonder why the .in file wasn't changed. or modify configure.ac and wonder why configure didn't work.

You're hacking around a fundamental property of autoconf for very little gain.

I for one, would like an option to turn off this "feature" as I wouldn't trust it with a 10 foot pole.

@hvr
Copy link
Contributor

hvr commented Aug 17, 2018

@Mistuke is right, my suggested solution only addresses part of the problem; specifically the part where you want to optimize the cabal.in -> cabal transformation for whe you're just messing with the .cabal.in files and you don't touch the configure files; but it's not something that occurs very frequently either...

...and once you modify the configure scripting, hadrian still ought to warn you that the build might be outdated; so you still need to implement something as suggested by @Mistuke in #655 (comment) regardless to tell the user they might need to re-run the configure phase.

@snowleopard
Copy link
Owner

snowleopard commented Aug 17, 2018

Yes, I agree. The two solutions are not mutually exclusive. Implementing @hvr's is easier and solves this particular issue, so I don't see any harm in implementing it immediately.

@Mistuke's proposal to introduce warnings for outdated configure-generated files is useful too, but it's not obvious to me what the right implementation is, so perhaps it deserves a separate issue.

@ndmitchell
Copy link
Collaborator Author

@snowleopard I think having configure be optional per run and wired through the build system is a bit of a disaster. I'd have it before shake starts up, and just check that the timestamp of the configure output is newer than the timestamp of the configure input. The --configure flag really changes the build system itself, so which you might be able to come up with a crazy encoding, it's not going to be fun.

I'd also minimize the problem using @hvr's suggestion.

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

No branches or pull requests

4 participants