Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 33 additions & 8 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,18 @@
##


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Fetch the version number from its source, in flexdll.opam

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

VERSION = 0.43
# Fetch the version number from its source, in flexdll.opam
VERSION = \
$(eval VERSION := $$(shell sed -ne 's/^version: *"\(.*\)"/\1/p' flexdll.opam))$(VERSION)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my education, what does the extra eval layer bring here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for all the details!
I had thought about laziness so I had tested VERSION = $(shell ... but I had called it only once 😄


all: flexlink.exe support

OCAML_CONFIG_FILE=$(shell cygpath -ad "$(shell ocamlopt -where 2>/dev/null)/Makefile.config" 2>/dev/null)
include $(OCAML_CONFIG_FILE)
OCAMLOPT=ocamlopt
EMPTY=
SPACE=$(EMPTY) $(EMPTY)
COMMA=,
OCAML_VERSION:=$(firstword $(subst ~, ,$(subst +, ,$(shell $(OCAMLOPT) -version 2>/dev/null))))
ifeq ($(OCAML_VERSION),)
OCAML_VERSION:=0
Expand All @@ -31,7 +35,7 @@ MIN64CC = $(MINGW64_PREFIX)gcc
CYGWIN64_PREFIX = x86_64-pc-cygwin-
CYG64CC = $(CYGWIN64_PREFIX)gcc

version.ml: Makefile
version.ml: Makefile flexdll.opam
echo "let version = \"$(VERSION)\"" > version.ml
echo "let mingw_prefix = \"$(MINGW_PREFIX)\"" >> version.ml
echo "let mingw64_prefix = \"$(MINGW64_PREFIX)\"" >> version.ml
Expand Down Expand Up @@ -160,11 +164,32 @@ flexlink.exe: $(OBJS) $(RES)
rm -f $@
$(RES_PREFIX) $(OCAMLOPT) -o $@ $(LINKFLAGS) $(OBJS)

version.res: version.rc
$(RES_PREFIX) rc $<

version_res.o: version.rc
$(TOOLPREF)windres -i $< -o $@
# VERSION at present is x.y, but there would be no reason not to have x.y.z in
# future. Windows versions have four components. $(FLEXDLL_FULL_VERSION) adds
# additional .0s to the right of $(VERSION) such that $(FLEXDLL_FULL_VERSION)
# has four version components.
# Thus if VERSION=0.43, then FLEXDLL_FULL_VERSION=0.43.0.0
# $(FLEXDLL_VS_VERSION_INFO) is the same value, but using a ',' to separate the
# items rather than a '.', as this is the format used in a VS_VERSION_INFO block
# in Resource Compiler format.
FLEXDLL_FULL_VERSION = \
$(subst $(SPACE),.,$(wordlist 1, 4, $(subst .,$(SPACE),$(VERSION)) 0 0 0))
FLEXDLL_VS_VERSION_INFO = $(subst .,$(COMMA),$(FLEXDLL_FULL_VERSION))

RC_FLAGS = \
/d FLEXDLL_VS_VERSION_INFO=$(FLEXDLL_VS_VERSION_INFO) \
/d FLEXDLL_FULL_VERSION="$(FLEXDLL_FULL_VERSION)"

# cf. https://sourceware.org/bugzilla/show_bug.cgi?id=27843
WINDRES_FLAGS = \
-D FLEXDLL_VS_VERSION_INFO=$(FLEXDLL_VS_VERSION_INFO) \
-D FLEXDLL_FULL_VERSION=\\\"$(FLEXDLL_FULL_VERSION)\\\"

version.res: version.rc flexdll.opam
$(RES_PREFIX) rc /nologo $(RC_FLAGS) $<
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, that would look much clearer (or perhaps "less unclear"!)


version_res.o: version.rc flexdll.opam
$(TOOLPREF)windres $(WINDRES_FLAGS) -i $< -o $@

flexdll_msvc.obj: flexdll.c flexdll.h
$(MSVC_PREFIX) $(MSVCC) /DMSVC -c /Fo"$@" $<
Expand Down Expand Up @@ -239,7 +264,7 @@ package_src:
rm -Rf flexdll-$(VERSION)
mkdir flexdll-$(VERSION)
mkdir flexdll-$(VERSION)/test
cp -a $(filter-out version.ml,$(OBJS:Compat.ml=Compat.ml.in)) Makefile msvs-detect $(COMMON_FILES) version.rc flexdll-$(VERSION)/
cp -a $(filter-out version.ml,$(OBJS:Compat.ml=Compat.ml.in)) Makefile msvs-detect $(COMMON_FILES) version.rc flexdll.install flexdll.opam flexdll-$(VERSION)/
cp -aR test/Makefile test/*.c flexdll-$(VERSION)/test/
tar czf $(PACKAGE) flexdll-$(VERSION)
rm -Rf flexdll-$(VERSION)
Expand Down
2 changes: 1 addition & 1 deletion appveyor_build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ if [ "$ARTEFACTS" = 'yes' ] ; then

make package_bin installer
SUFFIX="$(git describe)"
VERSION="$(sed -ne 's/^VERSION *= *//p' Makefile)"
VERSION="$(sed -ne 's/^version: *"\(.*\)"/\1/p' flexdll.opam)"
if [ "$SUFFIX" != "$VERSION" ] ; then
mv "flexdll-bin-$VERSION.zip" "flexdll-bin-$SUFFIX.zip"
mv "flexdll-$VERSION-setup.exe" "flexdll-$SUFFIX-setup.exe"
Expand Down
15 changes: 15 additions & 0 deletions flexdll.install
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
share: [
"Makefile"
"cmdline.ml"
"coff.ml"
"Compat.ml.in"
"create_dll.ml"
"default.manifest"
"default_amd64.manifest"
"flexdll.c"
"flexdll.h"
"flexdll.opam"
"flexdll_initer.c"
"reloc.ml"
"version.rc"
]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! late notice: shouldn’t there be flexdll.opam in there, since we’ll now need it to get the version number?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Confession: I didn’t try to build an OCaml compiler with these sources :-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😱 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.

11 changes: 11 additions & 0 deletions flexdll.opam
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
opam-version: "2.0"
version: "0.43"
authors: "Alain Frisch"
maintainer: "David Allsopp <[email protected]>"
bug-reports: "https://github.com/ocaml/flexdll/issues"
dev-repo: "git+https://github.com/ocaml/flexdll.git"
homepage: "https://github.com/ocaml/flexdll#readme"
license: "Zlib"
synopsis: "FlexDLL Sources"
description: "Source package for FlexDLL. Installs the required files for
bootstrapping FlexDLL as part of an OCaml build."
8 changes: 4 additions & 4 deletions version.rc
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ LANGUAGE LANG_ENGLISH, SUBLANG_ENGLISH_US
//

VS_VERSION_INFO VERSIONINFO
FILEVERSION 0,0,0,43
PRODUCTVERSION 0,0,0,43
FILEVERSION FLEXDLL_VS_VERSION_INFO
PRODUCTVERSION FLEXDLL_VS_VERSION_INFO
FILEFLAGSMASK 0x3fL
FILEFLAGS 0x0L
FILEOS 0x40004L
Expand All @@ -21,8 +21,8 @@ BEGIN
BEGIN
BLOCK "040904b0"
BEGIN
VALUE "FileVersion", "0.0.0.43"
VALUE "ProductVersion", "0.0.0.43"
VALUE "FileVersion", FLEXDLL_FULL_VERSION
VALUE "ProductVersion", FLEXDLL_FULL_VERSION
VALUE "ProductName", "FlexDLL"
VALUE "FileDescription", "FlexDLL Linker"
VALUE "LegalCopyright", "Institut National de Recherche en Informatique et en Automatique"
Expand Down