Allow users to vendor ppxlib as-is, as well as ppx_sexp_conv in the same project #386
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kit-ty-kate
requested review from
ceastlund,
panglesd and
pitag-ha
as code owners
kit-ty-kate
force-pushed
the
no-ppx_sexp_conv-vendor
branch
2 times, most recently
from
af67b43
to
a8bc98c
Compare
…ame project Signed-off-by: Kate <[email protected]>
kit-ty-kate
force-pushed
the
no-ppx_sexp_conv-vendor
branch
from
a8bc98c
to
9103cfe
Compare
pitag-ha
approved these changes
Feb 14, 2023
There was a problem hiding this comment.
Thanks a lot for the fix, @kit-ty-kate! This is a very good idea to fix the problem.
This was referenced Feb 14, 2023
kit-ty-kate
pushed a commit
to ocaml/opam-repository
that referenced
this pull request
Feb 14, 2023
CHANGES: - Allow users to vendor `ppxlib` as-is, as well as `ppx_sexp_conv` in the same project (ocaml-ppx/ppxlib#386, @kit-ty-kate)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This happens when e.g. using MirageOS 4+
This PR fixes this issue by making the vendored package part of the ppxlib package (no longer vendored from dune's point-of-vue). This way, when vendoring ppxlib,
ppx_sexp_convis no longer available in the "public namespace" of library names and there is no longer a clash between ppx_sexp_conv from ppxlib and ppx_sexp_conv from another vendored directory.Now that the ppx_sexp_conv is in the ppxlib namespace, all of its tests and so on were also triggered as part of ppxlib. This isn't desirable so I simply removed the
testdirectory.Tested successfully with mirage-skeleton (fails with 0.29.0, succeeds with this PR pinned)