mina
mina copied to clipboard
Remove unnecessary submodules (mini version)
Mini version of #15501.
Explain your changes:
- Remove infrequently updated submodules and copied-in repositories (except for
ocaml-rocksdb
, which building strategy makes it non-trivial)
Explain how you tested your changes:
- [x]
nix build 'git+https://github.com/MinaProtocol/mina?branch=georgeee/remove-unnecessary-submodules-mini&submodules=1#devnet'
to test nix build - [x] CI passes
Checklist:
- [x] Dependency versions are unchanged
- Notify Velocity team if dependencies must change in CI
- [ ] Modified the current draft of release notes with details on what is completed or incomplete within this project
- [x] Document code purpose, how to use it
- Mention expected invariants, implicit constraints
- [x] Tests were added for the new behavior
- Document test purpose, significance of failures
- Test names should reflect their purpose
- [x] All tests pass (CI will check this if you didn't)
- [x] Serialized types are in stable-versioned modules
- [x] Does this close issues? None
!ci-toolchain-me
!ci-build-me
!ci-toolchain-me
!ci-build-me
!ci-build-me
!ci-build-me
So the issue is the following:
git checkout origin/berkeley
./nix/pin.sh # just pins the submodules
git checkout origin/develop
./nix/pin.sh
git status
will give following output for the last command:
Untracked files:
(use "git add <file>..." to include in what will be committed)
src/external/
Repeating the steps:
git checkout origin/berkeley
./nix/pin.sh # just pins the submodules
removes this status, as expected.
If we switch back to develop and try to build the stuff:
git checkout origin/develop
./nix/pin.sh
nix develop mina
make build # from within nix shell
It succeeds though.
I suggest the following two remedies to avoid unnecessary friction:
- Add
src/external
to.gitignore
- Add
(data_only_dirs external)
tosrc/dune
This way we'll ensure neither git nor dune will ever see src/external
even if it happens to exist on file system after the switch of the branch.
Main concern brought up by Matthew is use of check_opam_switch
.
When berkeley
and develop
have divergent version version sets of library dependencies, this requires either use of two switches or rebuilding switch on each change of the branch. Which is of course not convenient.
This issue doesn't appear when working from within nix
shell, but is a problem otherwise.
The easiest solution that I see is to reject the idea of -mina
suffixes.
Option --set-default
in opam repository add --yes --all --set-default o1-labs https://github.com/o1-labs/opam-repository.git
should work just fine without this additional help of -mina
suffix.
And in nix it's even clearer that o1-labs repo takes precendence.
So -mina
suffixes does seem to be completely unnecessary, so reverting this change is just a natural way to resolve the issue.
And also now if one runs:
opam repository add --yes --all --set-default o1-labs https://github.com/o1-labs/opam-repository.git
opam update
opam switch import opam.export
once on feature branch (or future develop
) and then switches forth and back between feature branch and berkeley
, make check_opam_switch
succeeds without any additional action.
I'm not sure what the issue actually is at the moment. Is that only that this change would need OPAM users to have 2 switches?
If so, isn't it already an issue today if, says, someone has to update library dependencies in develop
. Or do we actually implicitly enforce some restrictions on library dependencies wrt to branching? (equality? subset inclusion master \in compatible \in develop).
I'm not sure what the issue actually is at the moment. Is that only that this change would need OPAM users to have 2 switches?
If so, isn't it already an issue today if, says, someone has to update library dependencies in develop. Or do we actually implicitly enforce some restrictions on library dependencies wrt to branching? (equality? subset inclusion master \in compatible \in develop).
This is an issue we have already, and it would raise for any PR changing the opam.export.
@mrmr1993's suggestion is to introduce some automation with the use of local switches to avoid unnecessary confusion when performing development in non-nix environment.
I guess this PR for now is blocked on us finding a way to auto-manage multiple switches.
!ci-toolchain-me
!ci-build-me
!ci-toolchain-me
!ci-build-me
!ci-build-me