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.

Sign up for GitHub

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

release pa_ppx.0.16 to support ocaml 5.3.0 syntax changes #27092

Merged
merged 11 commits into from
Dec 19, 2024
Merged

release pa_ppx.0.16 to support ocaml 5.3.0 syntax changes #27092

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
10768ec
release pa_ppx.0.16 to support ocaml 5.3.0 syntax changes
chetmurthy Dec 12, 2024
f40a7ab
exclude failing revdeps packages
chetmurthy Dec 13, 2024
f813099
maintainer-suggested cleanup
chetmurthy Dec 15, 2024
37f2ae6
push updated opam file
chetmurthy Dec 16, 2024
456f360
exclude pa_ppx.0.16
chetmurthy Dec 16, 2024
b18d4e9
back out capping pa_ppx_migrate, b/c can't build it anymore
chetmurthy Dec 16, 2024
d3a76b7
Merge branch 'master' into release-pa_ppx-for-ocaml-5.3.0
chetmurthy Dec 16, 2024
57f48ca
move conflicts over to the affected packages
chetmurthy Dec 16, 2024
84be58d
ignore CI failures on opensuse
chetmurthy Dec 16, 2024
2d3edfc
accept CI failures on opensuse-tumbleweed
chetmurthy Dec 17, 2024
88463e1
Update packages/pa_ppx/pa_ppx.0.16/opam
mseri Dec 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/pa_ppx/pa_ppx.0.16/opam
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ depends: [
conflicts: [
"ocaml-option-bytecode-only"
"pa_ppx_migrate" { <= "0.11" }
Copy link
Member

Choose a reason for hiding this comment

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

Sorry for not noticing this in earlier prs. Why are these added as conflicts instead of having the correct upper bounds in their respective opam files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

[I deleted two replies, b/c they were invalidated by the results of CI runs. I think this is an accurate answer]

For the bytecode-only option, a reviewer suggested it b/c (IIRC) there was no way to indicate to only build bytecode binaries on a platform that lacked native-code compiler.

For the others (e.g. for pa_ppx_migrate), I added them here b/c it seemed convenient. But when I moved the constraint pa_ppx < "0.16" to the file pa_ppx_migrate.0.11, the CI build for that package failed on Debian, b/c the package libpcre-dev is no longer available on Debian. The CI build failed when trying to build "conf-libpcre".

More modern versions of camlp5 and pa_ppx_* all rely on pcre2, not pcre, so that problem should be going away.

But this does bring up a mystery and maybe a problem: I have one package, pa_ppx_regexp that depends on all of re, pcre, and pcre2. B/c it allows the user to choose which "backend" to use for their regexps. I just released a new version of this package, which went thru fine (you approved it yesterday). So I don't know why that went thru.

Concretely, and this is an answer I would not have had before going thru this (ahem) "process of discovery", the reason maybe it's useful to put these constraints here in the new package, is that it means you don't have to build the old packages that you might have modified otherwise, and that can mean avoiding doing a bunch of spurious builds that might even fail for entirely unrelated reasons.

Whew!

Sorry, hope this is a good-enough answer. I'm writing this before the CI run completes, so hopefully I won't be deleting it and writing -another- reply. grin

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, the CI run passed, as it did before.

Copy link
Member

Choose a reason for hiding this comment

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

For the bytecode-only I agree, my question was concerning the packages. If they are incompatible with pa_ppx, I would drop conflicts here and move the upper bounds on the respective packages. It is easier for the solver to deal with them than via conflicts

Copy link
Member

Choose a reason for hiding this comment

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

We can do that in 2 stages. First add the relevant upper bounds on the other packages, then remove the conflicts from this. I can help in a couple of days if you want. And sorry again for not noticing it before

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I can move them, but then some of those respective packages will not build (or their deps/revdeps won't build -- is that OK?)

Copy link
Member

Choose a reason for hiding this comment

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

That's all right.

By the way, if you set an x-maintenance-intent field on your latest release, or if you flag the old ones as deprecated while you decide what you prefer to maintain, we can move them to the opam-repository-archive in the next stages of cleanup. See Plan and recent Announcement for more details

"pa_ppx_q_ast" { < "0.13" }
"pa_ppx_regexp" { < "0.03" }
]
build: [
[make "get-generated"]
Expand Down
Loading