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

solo5-bindings-* need conf-gcc #19755

Merged
merged 1 commit into from
Oct 13, 2021
Merged

Conversation

samoht
Copy link
Member

@samoht samoht commented Oct 12, 2021

No description provided.

@samoht
Copy link
Member Author

samoht commented Oct 12, 2021

Ok so ./configure.sh was broken for old gcc ... not sure if we want to fix this

(cf Solo5/solo5@18021b0#diff-0ba17b1855b0f97042c31b2b2c2fd2b075fa355a840b152f1959daf2a755266e)

@samoht
Copy link
Member Author

samoht commented Oct 12, 2021

so this PR is probably useless ... it's better to add a flag to say that's an expected error

@samoht samoht force-pushed the fix-solo5-bindings-hvt branch from cd7d796 to d4c9d7c Compare October 13, 2021 06:58
@samoht
Copy link
Member Author

samoht commented Oct 13, 2021

New tentative with a fix for all the solo5-bindings-* packages + using the avoid-version flag.

@samoht samoht changed the title solo5-bindings-hvt needs conf-gcc solo5-bindings-* need conf-gcc Oct 13, 2021
@dra27
Copy link
Member

dra27 commented Oct 13, 2021

There is a PR for opam-repo-ci to improve the error message, but what it's trying to tell you is that opam-version: "2.1" is incorrect - in all files where that's been changed it should be opam-version: "2.0" (there is no version 2.1 of the opam format).

The avoid-version flag is ignored by opam 2.0.x (as that's an opam 2.1 feature). If that's a problem then you either need to do a trick like the ocaml-beta package, add available: opam-version >= "2.1", or just live with it for opam 2.0.x (I expect that it's actually a case of just living with it?!)

@samoht
Copy link
Member Author

samoht commented Oct 13, 2021

Is opam 2.1 able to understand ignored flags in 2.0 files? If yes, I guess I can live with it (even if the CI will continue to fail :-/)

@dra27
Copy link
Member

dra27 commented Oct 13, 2021

No, opam 2.1 will interpret the flag

@dra27
Copy link
Member

dra27 commented Oct 13, 2021

In retrospect, the opam-version field is extremely unfortunately named - it is really the opam-format-version, so you're using the 2.0 specification of the format (which is what 2.1 uses). The flags field is part of opam 2.0 and the semantics are already that opam ignores flags it doesn't understand. So opam 2.1 will read and obey avoid-version, opam 2.0 will do nothing with it (it will display a warning if you turn on verbose output when updating the repo)

Also old versions have a broken configure.sh on gcc>=10 so are
not selected by default using the new 'avoid-version' flag
available in opam 2.1.
@samoht samoht force-pushed the fix-solo5-bindings-hvt branch from d4c9d7c to 2719bc9 Compare October 13, 2021 07:34
@samoht
Copy link
Member Author

samoht commented Oct 13, 2021

In retrospect, the opam-version field is extremely unfortunately named

I wonder who choose this name :p

@samoht
Copy link
Member Author

samoht commented Oct 13, 2021

Revdeps testing is going wild here, probably best to merge quickly..

@samoht samoht merged commit 0eaa0cc into ocaml:master Oct 13, 2021
@samoht samoht deleted the fix-solo5-bindings-hvt branch October 13, 2021 08:03
@avsm
Copy link
Member

avsm commented Oct 13, 2021

opam-version is about the same as (lang 2.0) in dune files. We just need to be consistent about explaining that these represent metadata format versions and not client versions.

@hannesm
Copy link
Member

hannesm commented Oct 13, 2021

imho this PR is wrong. solo5* works fine with clang. some old version do not work with gcc 10+ (see #19655 (comment))

@samoht
Copy link
Member Author

samoht commented Oct 13, 2021

Good point, I've reverted the conf-gcc part here: #19766

@dra27
Copy link
Member

dra27 commented Oct 13, 2021

In retrospect, the opam-version field is extremely unfortunately named

I wonder who choose this name :p

One who didn’t have the benefit of hindsight!! 🙇

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.

5 participants