Restore helpful error message when make is run before configure#11347
Closed
dra27 wants to merge 1 commit intoocaml:trunkfrom
Closed
Restore helpful error message when make is run before configure#11347dra27 wants to merge 1 commit intoocaml:trunkfrom
dra27 wants to merge 1 commit intoocaml:trunkfrom
Conversation
Contributor
|
Thanks a lot, David.
the error message definitely needs to be restored.
However, I do not feel comfortable with the way this is achieved,
although I don't yet see a better way.
What puzzles me is that I would like the dependencies we tell `make`
about are real, accurate ones. Only if we do that, I think, will we be
in a position where we can expect `make` to do its best job for us. I
don't think writing dependenceis jsut for having one thing occur before
another one is right (I am talking about the dependency of `sak` on
`configu.status` this PR introduces. To me this is not a genuine,
meaningful dependency.
I investigated a bit and found out that `make` tries to build `sak` very
eearly, even before it starts its real job, just because there is a
dependency of `stdlib/StdlibModueles` on `sak` and
`stdlib/StdlibModules` is included in the root Makefile.
This dependency of `stdlib/StdlibModules` is written and documented in
`Makefile.common` but I don't understand why exactly we need it?
I am willing to open another PR to fix all this in a way that feels more
straightforward to me, once we come to a good understanding.
|
dra27
added a commit
that referenced
this pull request
Jun 24, 2022
…or-message Restore helpful error message when make is run before configure (#11347 revisited)
Member
Author
The root |
Member
Author
|
Closing in favour of #11352, which is a cleaner approach! |
Contributor
|
David Allsopp (2022/06/24 00:50 -0700):
> This dependency of `stdlib/StdlibModules` is written and documented in `Makefile.common` but I don't understand why exactly we need it?
The root `Makefile` includes `stdlib/StdlibModules` and `sak` is
needed to build it.
To build what, sorry?
The dependency at present is there to ensure that _any_ inclusion of
`stdlib/StdlibModules` definitely builds sak first (the documentation
`Makefile`s also use it IIRC).
I was actually wondering whether the dependency couldn't be refined a
bit. Indeed, it's not the whole of `stdlib/StdlibModules` that requires
`sak`, but only the definition of the `STDLIB_MODULES` variable.
I expect this is something that becomes
much cleaner when all the `Makefile`s are merged - the problem at the
moment (or at least, as on the 4.14 branch) is that `runtime/Makefile`
is responsible for building `sak` but it's then used in lots of other
`Makefile`s
Regarding this, I am wondering whether I didn't overlook something while
merging `runtime/Makefile` in the root one. In `Makefile.common`, see
the rule that says:
```
$(ROOTDIR)/%/sak$(EXE):
$(MAKE) -C $(ROOTDIR)/$* sak$(EXE)
```
Shouldn't the recipe for this rule have been transformed into something
like
```
$(MAKE) -C $(ROOTDIR) $*/sak$(EXE)
```
?
If I am correct, shall I open a dedicated PR or may I just force-push to
trunk?
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Prior to 4.13, running
makein an empty tree displayed a helpful error message:Since 4.13, an alternate, much less helpful, error has been displayed. This PR restores the error message by adding the missing dependency on
config.statusto the generation rule forruntime/sak.$(O).