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

pull overwrites everything #348

Closed
hannesm opened this issue Nov 11, 2022 · 4 comments · Fixed by #366
Closed

pull overwrites everything #348

hannesm opened this issue Nov 11, 2022 · 4 comments · Fixed by #366

Comments

@hannesm
Copy link
Contributor

hannesm commented Nov 11, 2022

I use mirage, which uses opam-monorepo. Now I sometimes want to improve one package and test changes within a unikernel setup. How I achieve this is rm -rf duniverse/my-package ; cd duniverse ; ln -s ../../my-package . ; cd .. -- so removing the stuff duniverse pulled, and creating a link to my local development version.

Now, when I have 5 packages symlinked, eventually I want to revert some of them to the upstream version. What I attempt to do is rm duniverse/my-package - removing the symlink. Now, a opam monorepo pull removes all symlinks.

The question I have now: is there a better supported workflow to achieve the same? Is there a way to tell "opam monorepo" to only pull non-existing things, rather than overwriting everything?

@Leonidas-from-XIV
Copy link
Member

opam-monorepo operates with the assumption that it has the duniverse folder for itself, e.g. the dune file in there mentions that it will likely get overwritten (but so will other folders, there is no logic in place to deal with symlinks or preserving anything in there). This has the advantage that it always starts from a clean slate, no potentially hard to debug issues with stale files from e.g. previous pulls etc.

A possibility for the workflow that you're attempting is this: move the projects that you want to work on into a different folder, like vendor/ and then modify them there. This has the downside that on the next pull they will be pulled in again and dune will complain about duplicate definitions, so you'll have to remove them from the duniverse/ again. A potential workaround could be marking the packages you're working on as opam-provided but that might have unintended consequences for the dependencies of those packages, being also defined as opam-provided, so a different folder and occasional deleting seems to me like the most practical way to achieve your workflow.

@hannesm
Copy link
Contributor Author

hannesm commented Nov 11, 2022

Thanks for taking your time to answer this question. The outlined solutions look pretty strange to me. So is "opam-monorepo" not really meant for a workflow where dependencies are worked on? I'm still struggling with the more detailed semantics and workflows "opam-monorepo" attempts to solve.

@Leonidas-from-XIV
Copy link
Member

Personally having "pull creates a clean slate" is something I find extremely useful (and in fact being able to just nuke the installed packages and start from scratch instead if debugging the local switch or having to do a costly opam switch remove && opam switch create).

How would an option like opam-monorepo pull --preserve-symlinks sound to you? That would still require the repos you want to develop on to be symlinked (since there's no simple way to distinguish locally modified folders) but maybe that's an acceptable compromise? It's probably better to only have it symlinked anyway, since if a project is in duniverse/ then it has (vendored_dirs) enabled thus building like in release-mode.

@hannesm
Copy link
Contributor Author

hannesm commented Dec 23, 2022

Ok, that may work. But the question in the room still is whether a workflow like this is supported? Or how do people use "opam monorepo" with local changes?

The other question is what the semantics of "opam monorepo" is? Will it be able to figure out that my symlink from "my-dns-directory" is the same library/packages as the tarball from a "https://github.com/mirage/ocaml-dns.git" release?

As mentioned in #364 I still don't understand how "opam monorepo" is 'designed' to be used with local changes in different repositories (which I understood is the primary goal of such a monorepo approach) and how such local changes are to be upstreamed? Surprisingly, in my workflow I even sometiimes add and remove dependencies (thus my need to execute "opam monorepo" a second time)?

Leonidas-from-XIV added a commit to Leonidas-from-XIV/opam-monorepo that referenced this issue Dec 27, 2022
Leonidas-from-XIV added a commit to Leonidas-from-XIV/opam-repository that referenced this issue May 3, 2023
CHANGES:

### Added

- Display warning when a package to be locked is missing a `dev-repo` field and
  is being skipped because of it (tarides/opam-monorepo#341, tarides/opam-monorepo#362, @kit-ty-kate, @Leonidas-from-XIV)
- Add option `--keep-symlinked-dir` to preserve symlinks in `duniverse/`, which
  can be useful for local development. (tarides/opam-monorepo#348, tarides/opam-monorepo#366, @hannesm,
  @Leonidas-from-XIV)

### Changed

- Canonicalize the URLs of the OPAM `dev-repo` fields to be able to detect more
  semantically equivalent URLs, this should reduce the risk of build failures
  due to duplicate code pulled (tarides/opam-monorepo#118, tarides/opam-monorepo#365 @TheLortex, @Leonidas-from-XIV)

- Simple the error message printed when dependencies don't use dune as their
  build system. The opam-0install diagnostic message is no longer printed in
  this case and the message has been reformatted and reworded to make the
  salient information easier to see. (tarides/opam-monorepo#384, @gridbugs)

### Fixed

- Error in case where multiple packages with different dev-repo fields would be
  placed in the same duniverse directory (tarides/opam-monorepo#377, @gridbugs)

- Fix a failure when using opam-monorepo with an opam 2.2 root
  (tarides/opam-monorepo#379, @kit-ty-kate)

- Fix assertion failure when prefix of "lock" subcommand is used (tarides/opam-monorepo#381,
  @gridbugs)

- Treat packages without build commands as virtual only if also lack install
  commands, as some non-virtual packages might only have install commands.
  (tarides/opam-monorepo#376 @Leonidas-from-XIV, @gridbugs)
Leonidas-from-XIV added a commit to Leonidas-from-XIV/opam-repository that referenced this issue Sep 11, 2023
CHANGES:

### Added

- Display warning when a package to be locked is missing a `dev-repo` field and
  is being skipped because of it (tarides/opam-monorepo#341, tarides/opam-monorepo#362, @kit-ty-kate, @Leonidas-from-XIV)
- Add option `--keep-symlinked-dir` to preserve symlinks in `duniverse/`, which
  can be useful for local development. (tarides/opam-monorepo#348, tarides/opam-monorepo#366, @hannesm,
  @Leonidas-from-XIV)
- Adopt the OCaml Code of Conduct (tarides/opam-monorepo#391, @rikusilvola)
- Add solver tests (tarides/opam-monorepo#394, @samoht)

### Changed

- Canonicalize the URLs of the OPAM `dev-repo` fields to be able to detect more
  semantically equivalent URLs, this should reduce the risk of build failures
  due to duplicate code pulled (tarides/opam-monorepo#118, tarides/opam-monorepo#365 @TheLortex, @Leonidas-from-XIV)

- Simple the error message printed when dependencies don't use dune as their
  build system. The opam-0install diagnostic message is no longer printed in
  this case and the message has been reformatted and reworded to make the
  salient information easier to see. (tarides/opam-monorepo#384, @gridbugs)

- Encode `dev-repo` constraints in the opam solver - this allows to resolve
  more involved version constraints that were failing before (tarides/opam-monorepo#396, @samoht)

### Deprecated

### Fixed

- Error in case where multiple packages with different dev-repo fields would be
  placed in the same duniverse directory (tarides/opam-monorepo#377, @gridbugs)

- Fix a failure when using opam-monorepo with an opam 2.2 root
  (tarides/opam-monorepo#379, @kit-ty-kate)

- Fix assertion failure when prefix of "lock" subcommand is used (tarides/opam-monorepo#381,
  @gridbugs)

- Treat packages without build commands as virtual only if also lack install
  commands, as some non-virtual packages might only have install commands.
  (tarides/opam-monorepo#376 @Leonidas-from-XIV, @gridbugs)

- Improve the ordering of package candidates by putting broken packages at
  then end of the list (tarides/opam-monorepo#395, tarides/opam-monorepo#397, @samoht)

### Removed

### Security
Leonidas-from-XIV added a commit to Leonidas-from-XIV/opam-repository that referenced this issue Sep 11, 2023
CHANGES:

### Added

- Display warning when a package to be locked is missing a `dev-repo` field and
  is being skipped because of it (tarides/opam-monorepo#341, tarides/opam-monorepo#362, @kit-ty-kate, @Leonidas-from-XIV)
- Add option `--keep-symlinked-dir` to preserve symlinks in `duniverse/`, which
  can be useful for local development. (tarides/opam-monorepo#348, tarides/opam-monorepo#366, @hannesm,
  @Leonidas-from-XIV)
- Adopt the OCaml Code of Conduct (tarides/opam-monorepo#391, @rikusilvola)
- Add solver tests (tarides/opam-monorepo#394, @samoht)

### Changed

- Canonicalize the URLs of the OPAM `dev-repo` fields to be able to detect more
  semantically equivalent URLs, this should reduce the risk of build failures
  due to duplicate code pulled (tarides/opam-monorepo#118, tarides/opam-monorepo#365 @TheLortex, @Leonidas-from-XIV)

- Simple the error message printed when dependencies don't use dune as their
  build system. The opam-0install diagnostic message is no longer printed in
  this case and the message has been reformatted and reworded to make the
  salient information easier to see. (tarides/opam-monorepo#384, @gridbugs)

- Encode `dev-repo` constraints in the opam solver - this allows to resolve
  more involved version constraints that were failing before (tarides/opam-monorepo#396, @samoht)

### Deprecated

### Fixed

- Error in case where multiple packages with different dev-repo fields would be
  placed in the same duniverse directory (tarides/opam-monorepo#377, @gridbugs)

- Fix a failure when using opam-monorepo with an opam 2.2 root
  (tarides/opam-monorepo#379, @kit-ty-kate)

- Fix assertion failure when prefix of "lock" subcommand is used (tarides/opam-monorepo#381,
  @gridbugs)

- Treat packages without build commands as virtual only if also lack install
  commands, as some non-virtual packages might only have install commands.
  (tarides/opam-monorepo#376 @Leonidas-from-XIV, @gridbugs)

- Improve the ordering of package candidates by putting broken packages at
  then end of the list (tarides/opam-monorepo#395, tarides/opam-monorepo#397, @samoht)

### Removed

### Security
nberth pushed a commit to nberth/opam-repository that referenced this issue Jun 18, 2024
CHANGES:

### Added

- Display warning when a package to be locked is missing a `dev-repo` field and
  is being skipped because of it (tarides/opam-monorepo#341, tarides/opam-monorepo#362, @kit-ty-kate, @Leonidas-from-XIV)
- Add option `--keep-symlinked-dir` to preserve symlinks in `duniverse/`, which
  can be useful for local development. (tarides/opam-monorepo#348, tarides/opam-monorepo#366, @hannesm,
  @Leonidas-from-XIV)
- Adopt the OCaml Code of Conduct (tarides/opam-monorepo#391, @rikusilvola)
- Add solver tests (tarides/opam-monorepo#394, @samoht)

### Changed

- Canonicalize the URLs of the OPAM `dev-repo` fields to be able to detect more
  semantically equivalent URLs, this should reduce the risk of build failures
  due to duplicate code pulled (tarides/opam-monorepo#118, tarides/opam-monorepo#365 @TheLortex, @Leonidas-from-XIV)

- Simple the error message printed when dependencies don't use dune as their
  build system. The opam-0install diagnostic message is no longer printed in
  this case and the message has been reformatted and reworded to make the
  salient information easier to see. (tarides/opam-monorepo#384, @gridbugs)

- Encode `dev-repo` constraints in the opam solver - this allows to resolve
  more involved version constraints that were failing before (tarides/opam-monorepo#396, @samoht)

### Deprecated

### Fixed

- Error in case where multiple packages with different dev-repo fields would be
  placed in the same duniverse directory (tarides/opam-monorepo#377, @gridbugs)

- Fix a failure when using opam-monorepo with an opam 2.2 root
  (tarides/opam-monorepo#379, @kit-ty-kate)

- Fix assertion failure when prefix of "lock" subcommand is used (tarides/opam-monorepo#381,
  @gridbugs)

- Treat packages without build commands as virtual only if also lack install
  commands, as some non-virtual packages might only have install commands.
  (tarides/opam-monorepo#376 @Leonidas-from-XIV, @gridbugs)

- Improve the ordering of package candidates by putting broken packages at
  then end of the list (tarides/opam-monorepo#395, tarides/opam-monorepo#397, @samoht)

### Removed

### Security
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 a pull request may close this issue.

2 participants