I want to share some concerns about storing mix.lock file in VCS for libraries. The current version of Library Guidelines page has zero mentions of mix.lock and the established community practice is to indeed version control mix.lock.
Which is meaningless and might be even harmful in some scenarios.
Consider the library A that depends on another library B. B occasionally introduces a breaking change in the minor updates. Even if A library has nightly builds turned on, it continues to be green because in mix.lock there is a previous version of B. All the projects, depending on A get broken, because they ignore A’s mix.lock.
Without mix.lock in the VCS, A library author would have been informed a night after B upgrade. With mix.lock file in VCS, they will be informed by a tornado of issues from A users days after.
A less exotic case might be also taken into account: A depends on B and C, then B and C start conflicting on D at some point, but A nightly builds are still all green, because there is mix.lock in VCS and A is never tried to compile against modern versions of B, C, and D.
This is all not expected in our great self-organized ecosystem, but things happen. Currently, I personally switch nightly build on and remove mix.lock from VCS. It makes me think I will, at least, be notified about issues with the latest dependencies consistency the very next night.
I think removing mix.lock from VCS for libraries is a good practice in any case and I think we might enforce/suggest it via Guidelines I linked above.
The benefit of mix.lock is making builds reproducible. You don’t want contributors to jump into a library and then become unable to write a patch because they can’t get their tests to pass anyway.
If the goal is to catch bugs, then I recommend having additional builds on CI that remove the lock file before running. Then you get the best of both words.
If a new version of library A’s dependency B breaks A, and I’m some contributor trying to fix some other, unrelated bug in A, how does it benefit me or the owner or the owner of A that I’m stuck and can’t move forward?
If that were the only way to know about breaking updates to B then sure, but as Jose noted, this is solved with a CI setup that simply unlocks everything before running tests. Perhaps that suggestion is what is missing from the guides?
FWIW, I’ve found the same issue, and in CI I’ve started deleting the mix.lock before getting deps and running tests. to removing it for libraries; regardless, I’m hopeful that this isn’t a common problem.
mix deps.get has a command line option --check-locked. This was added in mix version 15.0.0. The doc says “raises if there are pending changes to the lockfile”. CI should use this so that if a change occurs where mix needs to modify the mix.lock file it can raise an exception. I recommend 1) check in the mix.lock file; 2) add that option to your CI builds. This ensures that what is built in CI is what was built on dev boxes and you’d be notified of discrepancies. mix deps.get options
If I understand correctly this makes your CI fail if there are possible updates to the dependencies, even if your CI ran otherwise fine with the latest updates (by deleting mix.lock in CI)?
Or do you not delete mix.lock and call mix deps.update --all in CI.
Because if you check in the mix.lock and just run mix deps.get --check-locked in CI then how can it fail?
Edit:
Well actually I don’t uderstant what raises if there are pending changes to the lockfile means. I though it was checking on git status but it seems that it’s supposed to do something else.
Let’s say someone makes a change to mix.exs to switch from version 1 to version 2 for a dependency. The lock file is now “out of date” as the version in the lock file is now incompatible with what’s in mix.exs.
In development that’s fine, and you just get deps to write a new lock file. but what if you developer forgets, and commits the new mix.exs version without pulling deps and writing a new lock file? This is the value of the option. In the CI you want this to raise instead of silently writing a lock file.
Well normally that is solved by simply having mix.lock checked in, and then everyone gets the same deps. BUT that isn’t reliable if someone can check in a mix.exs with an out of sync mix.lock. Then, the next dev to pull down the library code will mix deps.get and get a new lock file.
So it’s really the combination of this together:
Check in mix.lock so that deps are specified precisely and repeatably.
Use --check-locked in the CI to detect if the mix.exs file has drifted from the lockfile.
I understand now why using --check-locked is interesting (actually I just added it to my own CIs) but I don’t see how it changes the need to try to build after deleting the mix.lock to see if everything is fine.
Sorry I may be too tired today to understand all of it.
Oh ya I still think that the advice of having a CI task which deletes mix.lock and then tries to build / test is a great idea for libraries.
That said, I probably wouldn’t put it as a requirement for merging a PR. That is to say, if someone is trying to contribute to my library, I do expect the library to build and the tests to pass at the versions locked in the lock file. I want to know if maybe there’s some release out there which is going to require changes in my library, but that could be completely unrelated to the work of the contributor, and it isn’t reasonable to force them to solve that problem just because some dependency out there has a new version.
I would probably only run that additional task on main, so that I (and other contributors) can see that there is some maintenance work to be done to make sure it builds on the latest dep versions.
I’d even argue that such a test is most useful when run regularly (e.g. nightly) and not run coupled to developer activity. It’ll be far more likely to catch issues that way if the intention is to catch them before users do.
This may make perfect sense. I think if I wanted to enforce --check-locked on my open source library I’d put notes about it in CONˇRIBUTING.md (and CHANGELOG.md) and ask that the person making the PR check-in the modified mix.lock file.
When I work for a company I’ve added --check-locked for the applications CI. It’s helped reduce confusion and made engineers more aware of what’s going on.
to removing it for libraries; regardless, I’m hopeful that this isn’t a common problem.
