pkg: dedup package names in dependencies #10544
Conversation
gridbugs
force-pushed
the
fix-duplicated-depends
branch
from
cbf6435
to
1a86d73
Compare
gridbugs
force-pushed
the
fix-duplicated-depends
branch
2 times, most recently
from
2338a8d
to
b8f14c0
Compare
| else x :: ret_reversed, Package_name.Set.add seen x) | ||
| in | ||
| List.rev ret_reversed | ||
| ;; |
There was a problem hiding this comment.
It could potentially make sense to have a
val uniq : equal:('a -> 'a -> bool) -> 'a list -> 'a t
style function in Stdune.List, but this is fine too.
There was a problem hiding this comment.
Ah, I just noticed that List.sort_uniq already exists. It will obviously reshuffle the order, but might be ok for the usecase.
There was a problem hiding this comment.
Note that we aren't allowed to re-order the dependencies as specified by the user. The order has a semantic meaning.
There was a problem hiding this comment.
In the case of libraries yes, but do OPAM dependencies have a semantic ordering?
There was a problem hiding this comment.
I think we should preserve the order of dependencies in lockfiles if for no other reason than to simplify the mental model for the correspondence between lockfiles and the opam files they are generated from. If some fields are copied but reordered then it's more work to understand how the opam file maps to the lockfile.
|
Could you submit the failure in a separate commit? In this PR or a separate one. EDIT: didn't realize #10543 existed |
Fixes a bug where depending on a package with a constraint involving a conjunction would lead to the package appearing in the "depends" lockfile field multiple times. Signed-off-by: Stephen Sherratt <stephen@sherra.tt>
gridbugs
force-pushed
the
fix-duplicated-depends
branch
from
b8f14c0
to
00892e1
Compare
Fixes a bug where depending on a package with a constraint involving a conjunction would lead to the package appearing in the "depends" lockfile field multiple times. Signed-off-by: Stephen Sherratt <stephen@sherra.tt>
Fixes a bug where depending on a package with a constraint involving a conjunction would lead to the package appearing in the "depends" lockfile field multiple times.
Fixes #10542