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

stdlib-shims: fails to build on 4.12.0+trunk #16983

Merged
merged 4 commits into from
Aug 14, 2020
Merged

Conversation

avsm
Copy link
Member

@avsm avsm commented Aug 8, 2020

upstream fix proposed at ocaml/stdlib-shims#17

@dra27
Copy link
Member

dra27 commented Aug 8, 2020

This was fixed in Dune in ocaml/dune#3576

@avsm avsm closed this Aug 8, 2020
@avsm avsm reopened this Aug 8, 2020
@avsm
Copy link
Member Author

avsm commented Aug 8, 2020

Sounds like we need to conflict this with dune < 2.7 and ocaml <4.12 then

@avsm
Copy link
Member Author

avsm commented Aug 8, 2020

does 241ff77 look right @dra27?

@camelus
Copy link
Contributor

camelus commented Aug 8, 2020

Commit: e855240

A pull request by opam-seasoned @avsm.

☀️ All lint checks passed e855240
  • These packages passed lint tests: stdlib-shims.0.1.0

☀️ Installability check (+0)

@hannesm
Copy link
Member

hannesm commented Aug 9, 2020

I'm curious whether dune < 2.7.0 should require ocaml < 4.12.0, and not bother with constraining stdlib-shims at all... since it looks to me that more packages (all with empty modules) will encounter this issue...

@dra27
Copy link
Member

dra27 commented Aug 10, 2020

@hannesm is correct! Are you aware of any packages which would be affected by this change, though - it's not empty modules, but empty libraries (i.e. a library with no modules)

@avsm
Copy link
Member Author

avsm commented Aug 10, 2020

We'll find this out soon enough when we start doing bulk tests on 4.12. I suspect the current restriction is fine, and we can apply the big dune restriction if there turn out to be a non-trivial number of affected packages.

Co-authored-by: David Allsopp <david.allsopp@metastack.com>
@mseri
Copy link
Member

mseri commented Aug 13, 2020

Should we merge this or you prefer to wait? In the end it is a small change to keep the issue under control, it seems a reasonable start

@avsm
Copy link
Member Author

avsm commented Aug 14, 2020

Agreed. I'll merge go ahead and merge this, and it can be tested with #17005

@avsm avsm merged commit bceb815 into ocaml:master Aug 14, 2020
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.

6 participants