Skip to content

Show up for logs how we apply patches when we install ocaml-solo5 via opam#153

Closed
dinosaure wants to merge 2 commits intomainfrom
fix-compilation
Closed

Show up for logs how we apply patches when we install ocaml-solo5 via opam#153
dinosaure wants to merge 2 commits intomainfrom
fix-compilation

Conversation

@dinosaure
Copy link
Copy Markdown
Member

This patch does not change many things but it seems to fix the installation of ocaml-solo5 via opam into an ubuntu-latest GitHub machine. For the record, without this patch, I get an error (you can see here) and with this patch, it seems that we are able to install correctly ocaml-solo5 (you can see the output here - the error in the CI is unrelated to ocaml-solo5).

I suspect the set -x but it's highly related to #151. @shym do you have any insights?

@dinosaure
Copy link
Copy Markdown
Member Author

So we fallback to what we did about esperanto (for the context, see dinosaure/esperanto#52) and we must call git init -q. By this way, ocaml-solo5 is able to be compiled by dune pkg.

cp -r "$$(ocamlfind query ocaml-src)" $@
VERSION="$$(head -n1 ocaml/VERSION)" ; \
if test -d "patches/$$VERSION" ; then \
git apply --directory=$@ "patches/$$VERSION"/*; \
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just wondering: do all patches apply no matter the ordering? With patches/$VERSION/* I think we are not guaranteed any particular order, and it will depend on the filesystem. I see the patches go from 0001-*.patch..0021-*.patch.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, I think we need to force the order. I tested with the patches in reverse order and it fails:

$ git apply --check --directory ocaml patches/5.3.0/0021-Set-Max_domains_-def-max-to-1.patch patches/5.3.0/0020-Allow-ocaml-in-a-triplet-target-to-build-freestandin.patch patches/5.3.0/0019-Accept-native-freestanding-targets-at-configure-time.patch patches/5.3.0/0018-Output-the-.size-and-.type-directives-based-on-confi.patch patches/5.3.0/0017-Add-a-Config-variable-for-the-.size-and-.type-assemb.patch patches/5.3.0/0016-Detect-support-for-.size-and-.type-assembler-directi.patch patches/5.3.0/0015-Output-the-.note.GNU-stack-section-based-on-configur.patch patches/5.3.0/0014-Add-a-Config-variable-for-the-need-of-the-GNU-non-ex.patch patches/5.3.0/0013-Detect-the-need-for-the-GNU-note-for-non-executable-.patch patches/5.3.0/0012-Include-asm.h-at-the-beginning-of-assembler-sources.patch patches/5.3.0/0011-Add-a-dummy-caml_debugger_saved_instruction-when-HAS.patch patches/5.3.0/0010-Fix-static-builds-of-the-compiler.patch patches/5.3.0/0009-Enable-bootstrapping-flexdll-in-the-cross-compiler-s.patch patches/5.3.0/0008-Add-a-Makefile.cross-with-rules-to-build-a-cross-com.patch patches/5.3.0/0007-Detect-flexlink-only-on-relevant-targets.patch patches/5.3.0/0006-Add-a-configurable-library-directory-on-target.patch patches/5.3.0/0005-Use-a-TARGET_BINDIR-configure-variable-instead-of-wi.patch patches/5.3.0/0004-Check-that-the-OCaml-versions-are-compatible-for-a-c.patch patches/5.3.0/0003-Detect-a-_build_-C-toolchain-to-build-sak.patch patches/5.3.0/0002-Use-target-instead-of-host-when-relevant-in-configur.patch patches/5.3.0/0001-Use-target-instead-of-host-to-detect-the-C-toolchain.patch
error: the patch applies to 'ocaml/configure' (401b94298e4f34946677a409560ce1832c5bbbbc), which does not match the current contents.
error: ocaml/configure: patch does not apply
error: patch failed: ocaml/configure.ac:299
error: ocaml/configure.ac: patch does not apply

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Indeed the order must be enforced. I wrote it that way because IIUC sh is expected (by POSIX) to sort entries when expanding wildcards. Now I discover bash parameter $GLOBSORT that can be used to control (including disabling) how sorting is done, so this is another brittleness of that patching code. Sigh.
Anyway, in the log I don’t see any failure due to some patch not applying, so I don’t think that the order of the patches is causing issues.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

FWIW I got the same error message when I did git apply --check directory ocaml patches/5.3.0/* and adding echo in front it's in the numerical order. So I don't know what's going on and why I'm getting that error.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I still think that using git apply is not appropriate because it also looks at .git/index. It is much more than just a patch between two files; it is a patch between two files and the state of the Git repository we are in (and it is difficult to know how git apply finds .git).

In short, I think we're back to the initial issue described here: #151

@shym
Copy link
Copy Markdown
Contributor

shym commented Sep 22, 2025

This patch does not change many things but it seems to fix the installation of ocaml-solo5 via opam into an ubuntu-latest GitHub machine. For the record, without this patch, I get an error (you can see here) and with this patch, it seems that we are able to install correctly ocaml-solo5 (you can see the output here - the error in the CI is unrelated to ocaml-solo5).

I suspect the set -x but it's highly related to #151. @shym do you have any insights?

As opam hides the successful log, it’s hard to compare the logs.
(I’m not sure what you have in mind about set -x there.)

So, what is strange is that git isn’t complaining that patches cannot be applied (I just checked that the error appears even without -v, fortunately, at least in my local version of git; but I don’t expect such errors to be hidden without -v anyway). And still, the error in the ./configure call is clearly signalling that configure wasn’t properly patched.

What comes to mind:

  • We might turn the ; into a && between VERSION=... and for... to make sure the VERSION file was indeed read properly; and maybe log it explicitly? I don’t see how it could be the case there, but an empty VERSION could explain what we saw.
  • I look forward to OCaml 5.4, where we could decide to get completely rid of patches: only configure and the default number of domains will need to be changed, maybe we should go there with a simple cp instead of trying to be clean.

@hannesm
Copy link
Copy Markdown
Member

hannesm commented Nov 20, 2025

is this superseeded by #154?

@dinosaure
Copy link
Copy Markdown
Member Author

Yes, thanks for the work.

@dinosaure dinosaure closed this Nov 20, 2025
@hannesm hannesm deleted the fix-compilation branch November 20, 2025 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants