Skip to content

Fix the no-naked-pointers GitHub run on release tags#10173

Merged
xavierleroy merged 1 commit intoocaml:trunkfrom
dra27:Octachron's-heart
Jan 27, 2021
Merged

Fix the no-naked-pointers GitHub run on release tags#10173
xavierleroy merged 1 commit intoocaml:trunkfrom
dra27:Octachron's-heart

Conversation

@dra27
Copy link
Copy Markdown
Member

@dra27 dra27 commented Jan 26, 2021

The GitHub Actions runs for the 4.12 release tags have been failing, although unnoticed until it caused unnecessary stress for our release manager, @Octachron with today's!

The no-naked-pointers workflow predates the others - it's a very specific check which when multicore's merged will disappear. This PR reduces the time spent on that job with --disable-dependency-generation (present in the other jobs apart from the full-flambda job, where dependency generation is meant to be tested) and, more importantly, adds --enable-ocamltest so the future release tags will pass CI.

Running through CI just to make sure, and will then push to trunk and 4.12.

Add --disable-dependency-generation, since the no-naked-pointers run is
meant to run as quickly as possible.

Ensure --enable-ocamltest is added, as in the other jobs, to allow the
testsuite to run when building release tags and lower our release
manager'se heart rate!
Copy link
Copy Markdown
Member

@Octachron Octachron left a comment

Choose a reason for hiding this comment

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

I wholeheartedly approve the idea of not having tests that fails automatically on release commits.

@xavierleroy xavierleroy merged commit 4008866 into ocaml:trunk Jan 27, 2021
@xavierleroy
Copy link
Copy Markdown
Contributor

I wholeheartedly approve the idea of not having tests that fails automatically on release commits.

So do I. But I'm increasingly doubtful that it's a good idea to have different configure defaults for "release" and "development" builds. That's a recipe for problems at release time.

dra27 added a commit that referenced this pull request Jan 28, 2021
Ensure --enable-ocamltest is added, as in the other jobs, to allow the
testsuite to run when building release tags.

Add --disable-dependency-generation, since the no-naked-pointers run is
meant to run as quickly as possible.

(cherry picked from commit 4008866)
@dra27 dra27 deleted the Octachron's-heart branch January 28, 2021 10:21
@dra27
Copy link
Copy Markdown
Member Author

dra27 commented Jan 28, 2021

Cherry-picked to 4.12 as well.

Regarding the problem, --disable-ocamltest was added in #9250 to eliminate downstream patches when building cross-compilers (specifically in the riscv-cross, but I believe it affects all of them). The error message when running the testsuite on a release tag was improved in #9935. The machinery for treating release tags differently was added in 4.03.0 for C warnings-as-errors, which this simply piggy-bags onto.

On a quiet 16 core Xeon Silver 4108, ocamltest takes 2 seconds to build with make -j. It may not the biggest crime in our carbon-consuming ecosystem, but having spotted in #9250 that we spend a non-trivial amount of time building a tool which is neither used nor installed in release builds, I'd feel environmentally dirty if we go back to doing that (and I am of course gradually doing rather bigger things to reduce the number of times our beloved compiler is compiled every day by the community).

The testsuite already builds testsuite/lib and testsuite/tools on first run. We could do the same for ocamltest (IIRC that was discussed around #9935, but was dismissed in terms of a simpler warning)?

It's also possible with the various testing infrastructure to push the release commits (i.e. 53094bb and c6da7e0) but not the tag. opam-repository/bulk-builds/opam-health-check can all be set to run against the commit instead of the tag which would run all the tests in release mode. At that point the tag can be pushed (local CI then catches up, but is largely irrelevant). If a problem is found, VERSION can be set back after it's fixed (because we didn't tag it).

dra27 added a commit to dra27/ocaml that referenced this pull request Sep 6, 2022
Ensure --enable-ocamltest is added, as in the other jobs, to allow the
testsuite to run when building release tags.

Add --disable-dependency-generation, since the no-naked-pointers run is
meant to run as quickly as possible.

(cherry picked from commit 4008866)
dra27 added a commit to dra27/ocaml that referenced this pull request Sep 6, 2022
Ensure --enable-ocamltest is added, as in the other jobs, to allow the
testsuite to run when building release tags.

Add --disable-dependency-generation, since the no-naked-pointers run is
meant to run as quickly as possible.

(cherry picked from commit 4008866)
dra27 added a commit to dra27/ocaml that referenced this pull request Sep 6, 2022
Ensure --enable-ocamltest is added, as in the other jobs, to allow the
testsuite to run when building release tags.

Add --disable-dependency-generation, since the no-naked-pointers run is
meant to run as quickly as possible.

(cherry picked from commit 4008866)
dra27 added a commit to dra27/ocaml that referenced this pull request Sep 6, 2022
Ensure --enable-ocamltest is added, as in the other jobs, to allow the
testsuite to run when building release tags.

Add --disable-dependency-generation, since the no-naked-pointers run is
meant to run as quickly as possible.

(cherry picked from commit 4008866)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants