-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Various build cleanups #13112
Various build cleanups #13112
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice clean up!
I鈥檝e pushed on my cleanups-suggest branch two small suggestions:
- to go back to flexdll and winpthreads configuration code and remove the variables rendered nicely obsolete by this PR,
- to drop cleaning of winpthreads submodule now that it is built out-of-tree.
In particular, this means the Makefile fragments (-I $(ROOTDIR)/flexdll etc) do not get put into ocamltest/ocamltest_config.ml
Makefile.build_config isn't loaded for make clean
As for flexdll-sources
Philosophical fix - flexlink is unconditionally rebuilt from the .ml files if the executable is missing, but make clean should actually clean objects.
The way the winpthreads submodule has been extracted from mingw-w64 means that there is no .gitignore for objects created in it which is a git-status nuisance when developing. Fix that by instead placing the objects directly in the runtime/ directory.
Also removes a few obsolete variables
Sorry for the lag. Dropping the clean statement was in my mind, but apparently I then didn't do it, so thanks for that one. For flexdll, the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of these things were really bugging me, thanks for the fixes!
LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved on @MisterDA's behalf.
Gabriel Scherer (2024/05/15 02:50 -0700):
Merged #13112 into trunk.
Thanks. In general I'd be happy to get a chance to review these things.
|
This PR collects a whole load of related tweaks together which started from some checking done in #13093.
configure.ac
which I should have put in Overhaul the FlexDLL bootstrap聽#10135 originally.configure.ac
andMakefile
for these two submodules - i.e.configure.ac
stops generatingMakefile
-fragments and simply emits strings, like it's supposed to..gitignore
entry we missed in Restore the MSVC port of OCaml聽#12954 (we also missed cleaningwinpthreads-sources
indistclean
which is fixed a few commits later).yacc/wstr.obj
(a Windows-specific object) didn't work because it relies onMakefile.build_config
which isn't loaded in a clean target. That's also a bit of meta-programming / layer violation between configure/make which I'm not terribly fond of, but I don't have a better solution to hand at the moment.make distclean
from an unconfigured tree.make clean
ought to clean theflexdll
objects, which is done in the next commit (it's only morally becausemake clean
does clean the flexlink executable and for reasons of history flexdll's Makefile always compiles by specifying the.ml
files on the command line, rather than the more traditional separate compilation and linking, so flexlink was always completely recompiled after amake clean
)..obj
files. I've fixed that by instead putting the winpthreads objects inruntime/winpthreads
during the build.cc @shindere, @shym, @MisterDA 馃檪