opam package for flexdll sources #135
Conversation
|
The opam package can in fact be tested from Linux with: $ opam pin add git+https://github.com/dra27/flexdll.git#flexdll-source-packaging
...
$ ls -l $(opam var flexdll:share)
# All the source files needed for flexdll bootstrapping |
dra27
force-pushed
the
flexdll-source-packaging
branch
2 times, most recently
from
c98405e to
7459cfc
Compare
shym
left a comment
shym
left a comment
There was a problem hiding this comment.
LGTM, though I’m not sure about resource formats.
To try and ensure that the version numbers are correctly set up, I wanted to look at the resources in the binary files. I’m not sure about the proper way to do that, so I’ve opened in Visual Studio:
flexlink.exegenerated by the MinGW toolchain (so thewindrestool for the resources),version.resgenerated byrc,
and they show the same version numbers. On the other hand, the byte sequences in the files version_res.o and version.res are different: in version_res.o, FileVersion is really encoded as a string, but it is not in version.res. I don’t know if that really matters.
|
|
||
| VERSION = 0.43 | ||
| VERSION = \ | ||
| $(eval VERSION := $$(shell sed -ne 's/^version: *"\(.*\)"/\1/p' flexdll.opam))$(VERSION) |
There was a problem hiding this comment.
For my education, what does the extra eval layer bring here?
There was a problem hiding this comment.
It's a GNU make pattern for lazy variables. The problem with VERSION := $(shell ...) is that it gets run every time the Makefile is loaded and with VERSION = $(shell ...) the problem is it gets run every time $(VERSION) is used.
The first time $(VERSION) is expanded, it evaluates VERSION := $(shell ...) meaning that $(VERSION) is now the result of the $(shell ...) call. However, := doesn't expand to anything, so the final $(VERSION) ensures that the expansion actually returns something. Future expansions of $(VERSION) are now constants.
There was a problem hiding this comment.
Thank you for all the details!
I had thought about laziness so I had tested VERSION = $(shell ... but I had called it only once 😄
| -D FLEXDLL_FULL_VERSION=\\\"$(FLEXDLL_FULL_VERSION)\\\" | ||
|
|
||
| version.res: version.rc flexdll.opam | ||
| $(RES_PREFIX) rc /nologo $(RC_FLAGS) $< |
There was a problem hiding this comment.
This is not due to this PR but I find it a bit confusing to have *_PREFIX variable names both for cases when they are to be glued to the actual commands (CYGWIN64_PREFIX say) and cases when they are not (MSVC64_PREFIX and, here, RES_PREFIX).
Maybe the not-to-be-glued variables could be named *_EXTRA_ENV or they could add the needed space explicitly (ie end up with ... $(EMPTY)) so they are glued when used?
There was a problem hiding this comment.
I agree, that would look much clearer (or perhaps "less unclear"!)
| "flexdll_initer.c" | ||
| "reloc.ml" | ||
| "version.rc" | ||
| ] |
There was a problem hiding this comment.
Oh! late notice: shouldn’t there be flexdll.opam in there, since we’ll now need it to get the version number?
There was a problem hiding this comment.
(Confession: I didn’t try to build an OCaml compiler with these sources :-)
There was a problem hiding this comment.
😱 Eek - I hadn't either, I'm very glad you spotted this!
C:\Devel>opam pin add flexdll git+https://github.com/dra27/flexdll.git#flexdll-source-packaging
[flexdll.0.43] synchronised (git+https://github.com/dra27/flexdll.git#flexdll-source-packaging)
flexdll is now pinned to git+https://github.com/dra27/flexdll.git#flexdll-source-packaging (version 0.43)
The following actions will be performed:
=== recompile 6 packages
↻ base-domains base [uses ocaml]
↻ base-nnp base [uses base-domains]
↻ flexdll 0.43 (pinned)
↻ ocaml 5.1.1 [uses ocaml-base-compiler]
↻ ocaml-base-compiler 5.1.1 [uses flexdll]
↻ ocaml-config 3 [uses ocaml-base-compiler]
Proceed with ↻ 6 recompilations? [y/n] y
<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><> 🐫
⊘ removed base-nnp.base
⊘ removed base-domains.base
⊘ removed ocaml.5.1.1
⊘ removed ocaml-config.3
⬇ retrieved ocaml-base-compiler.5.1.1 (cached)
⊘ removed ocaml-base-compiler.5.1.1
⊘ removed flexdll.0.43
∗ installed flexdll.0.43
[ERROR] The compilation of ocaml-base-compiler.5.1.1 failed at "make -j19".
#=== ERROR while compiling ocaml-base-compiler.5.1.1 ==========================#
# context 2.2.0~beta2~dev | win32/x86_64 | | git+file://C:/Devel/opam-repository-3#windows-initial
# path ~\AppData\Local\opam\test-5.1.1\.opam-switch\build\ocaml-base-compiler.5.1.1
# command ~\AppData\Local\opam\.cygwin\root\bin\make.exe -j19
# exit-code 2
# env-file ~\AppData\Local\opam\log\ocaml-base-compiler-42868-728b84.env
# output-file ~\AppData\Local\opam\log\ocaml-base-compiler-42868-728b84.out
### output ###
# [...]
# /usr/bin/make -C flexdll-sources MSVC_DETECT=0 OCAML_CONFIG_FILE=../Makefile.config CHAINS=mingw64 ROOTDIR=.. \
# OCAMLRUN='$(ROOTDIR)/boot/ocamlruns.exe' NATDYNLINK=false \
# OCAMLOPT='$(OCAMLRUN) $(ROOTDIR)/boot/ocamlc -use-prims ../runtime/primitives -nostdlib -I ../stdlib' \
# -B flexlink.exe support
# make[3]: Entering directory '/cygdrive/c/Users/DRA/AppData/Local/opam/test-5.1.1/.opam-switch/build/ocaml-base-compiler.5.1.1/flexdll-sources'
# make[3]: *** No rule to make target 'flexdll.opam', needed by 'version.ml'. Stop.
08c092b to
d4841d2
Compare
|
Thanks, @shym! I've re-ordered the commits to better reflect what's actually happening (and what I had been testing)... the .install file is correct for FlexDLL 0.43, so I've moved the bit to harmonise the version number to the last commit (which will ultimately be FlexDLL 0.44). |
This file can be referenced from the opam packages for these pre-releases to be used in addition to the tarball.
The flexdll package is intended for use when building OCaml in "bootstrapped" flexdll mode and installs the required source files to the share folder in an opam switch. The .install can be used with both FlexDLL 0.42 and FlexDLL 0.43
dra27
force-pushed
the
flexdll-source-packaging
branch
from
d4841d2 to
cfd82ea
Compare
shym
left a comment
shym
left a comment
There was a problem hiding this comment.
I went through the changes again, this is good to merge I think.
Maybe we could add a comment (for our future selves 😅) that the version number is now only stored in flexdll.opam.
| ## make all MSVC_DETECT=0 CHAINS="mingw mingw64 cygwin64 msvc64" | ||
| ## | ||
|
|
||
|
|
There was a problem hiding this comment.
| # Fetch the version number from its source, in flexdll.opam |
The VERSION macro in Makefile is now the single source of the version of FlexDLL. This is used when compiling version.rc. version.rc is corrected to adopt Windows version format, so the current release is rendered as 0.43.0.0 instead of 0.0.0.43. flexdll.opam is now the primary source for the version number for the package, and both version.ml and version.rc are compiled using the number taken from there.
dra27
force-pushed
the
flexdll-source-packaging
branch
from
cfd82ea to
c5d59b5
Compare
|
Out of an abundance of caution brought on by that error in the .install, I have tested the opam packaging of 4.14.1 with 0.36-0.43 (which use tarballs, and grab the .install files from these commits) and then pinned this branch as well! Out of even more caution, I quickly upgraded ocaml-base-compiler.4.05.0 and confirmed that the 0.35 source package works too! |
|
I'll merge this once CI catches up - thank you very much for the very careful reviewing! |
2 participants
This is the first commit from #111, split into two sub-commits for slightly easier reviewing. This deals with OCaml 4.03+, which is all that's needed for the first batch of opam-repository updates.