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

Allow resolving unprefixed refs #332

Merged

Conversation

Leonidas-from-XIV
Copy link
Member

When using a pinned dependency the refs might not have a prefix (e.g. HEAD is a plain ref) thus they wouldn't have been found when attempting to resolve the refs.

Closes #326

@samoht
Copy link
Collaborator

samoht commented Nov 2, 2022

Could you add a test-case to ensure we don't introduce future regressions there? I'm not sure I understand this logic either.

When using a pinned dependency the refs might not have a prefix (e.g.
`HEAD` is a plain ref) thus they wouldn't have been found when
attempting to resolve the refs.

Closes tarides#326
@Leonidas-from-XIV Leonidas-from-XIV force-pushed the resolve-unprefixed-refs branch 2 times, most recently from d67a54b to 1091af2 Compare November 21, 2022 14:50
@Leonidas-from-XIV
Copy link
Member Author

@samoht I've added a test. It's a bit ugly since it messes with the global OPAM state (by creating a pin called opam-monorepo-test-other but the original report can only be reproduced with pins.

@Leonidas-from-XIV Leonidas-from-XIV merged commit 5382d1a into tarides:main Nov 24, 2022
@Leonidas-from-XIV Leonidas-from-XIV deleted the resolve-unprefixed-refs branch November 24, 2022 09:07
Leonidas-from-XIV added a commit to Leonidas-from-XIV/opam-repository that referenced this pull request Dec 21, 2022
CHANGES:

### Changed

- Treat packages with no build commands as if they can be built with dune (tarides/opam-monorepo#355,
  @gridbugs)

### Fixed

- Fix resolving refs of locally pinned repositories (tarides/opam-monorepo#326, tarides/opam-monorepo#332, @hannesm,
  @Leonidas-from-XIV)
- Read the `compiler` flag from OPAM metadata thus classifying more packages
  correctly as base packages (tarides/opam-monorepo#328, @Leonidas-from-XIV)
- Fix bug where dev repo urls ending with a "/" would result in
  `opam monorepo pull` placing package source code directly inside the duniverse
  directory instead of in a subdirectory of the duniverse directory (tarides/opam-monorepo#359,
  @gridbugs)
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.

failure with a pinned opam package
3 participants