Skip to content
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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test building ocamlnat on Inria's CI #8991

Closed
wants to merge 1 commit into from

Conversation

shindere
Copy link
Contributor

This pull request is a follow-up to an offline discussion with
@Armael and @Octachron, itself related to issue #8989.

All three of us agreed that if the native toplevel is distributed
with the compiler, it should at least compile.

I thus went ahead and added it to Inria's CI to see whether it
compiles everywhere, as @xavierleroy wondered.

As shown by precheck's build
#304(https://ci.inria.fr/ocaml/job/precheck/build/304) it turns out
that trunk's ocmalnat does compile on all architectures tested
by Inria's CI (there are a few failures but I made sure they are
unrelated).

This PR thus proposes to add this very minimal test to our CI
infrastructure. A later PR could then introduce a very simple
execution test to make sure problems like those fixed by @Octachron
in PR #8988 can be caught earlier.

Obviously, the goal of this PR is to agree on the principle, rather than
on the implementation. :)

@xavierleroy
Copy link
Contributor

I'm not too surprised that ocamlnat compiles everywhere. But does it run?

@shindere
Copy link
Contributor Author

shindere commented Sep 27, 2019 via email

@XVilka
Copy link
Contributor

XVilka commented Sep 30, 2019

By the way, have you considered to show the INRIA CI status in the GitHub commits and pull requests? As I can see it's Jenkins based, which can be integrated with GitHub easily:

@shindere
Copy link
Contributor Author

shindere commented Oct 24, 2019 via email

@shindere
Copy link
Contributor Author

Can we please make a decision on that one?

@gasche
Copy link
Member

gasche commented Oct 24, 2019

decision: no, unless the test is part of a "it's okay to break this" category that doesn't make the testsuite send big red warnings to everyone when they are broken. See my earlier comment in the other thread: we have collectively decided to not care about ocamlnat breakage for now.

@Octachron
Copy link
Member

There are breakages and breakages. If we don't even care about the fact that ocamlnat merely builds, why keep it in the repository?

@gasche
Copy link
Member

gasche commented Oct 25, 2019

Personally I think that native toplevel is a nice thing to have (although I have never used it myself, and the people for which it could make sense (hol, coq) are surprisingly un-enquiring about it). I would be happy to see it as a supported part of the ecosystem. (I know that other people are interested in doing even more work in that direction, including having the ocaml compiler replace the assembler and generate valid binary for dynlinking or just-in-time code generation...) However, we collectively agreed that the maintainers would not pay the cost of keeping ocamlnat working on trunk, only a few self-selected people would update it from time to time.

If you are interested in keeping ocamlnat healthy, I think it would be helpful to add yourself to the set of self-selected people that maintain it. Demonstrating over time that it's easy to maintain could let us have a collective conversation again, and maybe decide to change the statu quo. Having more people interested would also justify changing our CI processes to make things easier for them, and possibly over time shifting that responsibility to everyone.

The present suggestion is to not test that ocamlnat works, not notify contributors that their PRs are breaking ocamlnat's build, not have more people maintain it, but to annoy everyone that is subscribed to INRIA's CI notifications when the build breaks. The hope is that it will motivate someone to fix it (I guess?), and the risk is that it makes people ignore INRIA's CI because it sends still-broken emails on each merge. I don't think it would be an improvement.

@shindere
Copy link
Contributor Author

shindere commented Oct 25, 2019 via email

@shindere
Copy link
Contributor Author

shindere commented Oct 25, 2019 via email

@mshinwell
Copy link
Contributor

Could we find out (say by running some kind of test job, or manually sshing into the various CI slaves) how the execution of ocamlnat goes on the various platforms? If it's basically working, then maybe we could just treat it the same as the other compiler tools, and build and test it normally. I can't help but wonder if we're spending more time on discussion (and potentially on maintaining extra infrastructure) than it would take to keep it working :)

@shindere
Copy link
Contributor Author

shindere commented Oct 25, 2019 via email

@xavierleroy
Copy link
Contributor

Oooooooooordeeeeeeeeeeeeer! (Bercow-style)

A while ago, we (core OCaml development team) decided collectively to keep the ocamlnat code around as an experiment, but not support it officially.

Until this decision is rescinded, we do not support ocamlnat. This implies that we do not test ocamlnat at all -- not even "does it still compile?" -- in the standard CI, at Inria and on Appveyor and Travis.

People who are not happy about this decision can set up their own CI for ocamlnat, or argue for a different decision at the next developers' meeting.

My personal take is that we have more urgent things to do than supporting ocamlnat, both in terms of new featuers that need much integration work (Multicore OCaml, anyone?) and in terms of CI (which is unacceptably unreliable and slow already).

@damiendoligez
Copy link
Member

I think a once-a-day ocamlnat-specific CI job that only spams self-selected people is an acceptable middle ground.

@shindere
Copy link
Contributor Author

shindere commented Mar 31, 2020 via email

@dra27
Copy link
Member

dra27 commented Aug 4, 2023

This PR was rendered moot by #10690 - the equivalent line added in this PR (which was already present in the GitHub actions runner script) was removed as part of it (cf. b64a764#diff-eb1ad9c46850a3ae937d62938dad230b22050cb74cab92d0cb3582f4c53793e5L67)

@dra27 dra27 closed this Aug 4, 2023
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.

None yet

8 participants