Fix dep loader rejecting fetched deps with stale .app#15406
Open
ericmj wants to merge 2 commits into
Open
Conversation
When a fetchable dependency is re-fetched at a new version, the source
in `deps/<name>` is replaced but the build artifacts under
`_build/<env>/lib/<name>` are not. The dep loader's `validate_app`
reads the stale `_build/.../<name>.app`, sees the previous version
string, and either reports `:nomatchvsn` against the new requirement
or, more subtly, returns `{:ok, old_vsn}` which then trips the
converger's `req_mismatch` check and surfaces as `:divergedreq`.
This used to be guarded by `recently_fetched?`, which short-circuited
`validate_app` to `:compile` whenever a `.fetch` marker (later
`compile.fetch` then `compile.elixir_scm`) indicated the build was
behind the source. That guard was removed in "Store the lock in the
manifest" in favor of `check_manifest` comparing the stored lock
against the current lock and marking the dep as `:compile`. But
`check_manifest` is only reached via `check_lock`, which is guarded by
`available?(dep)` — and once `validate_app` has produced `{:ok, _}`,
the converger can set `:divergedreq` first, which makes the dep
diverged and `available?` false, so the manifest lock check never
fires.
Restore the short-circuit in `validate_app` using the new signal: if
the SCM manifest's stored lock differs from `opts[:lock]` (or the
manifest is missing), treat the dep as `:compile` and skip validating
the stale `.app`. This preserves the architecture introduced by the
lock-in-manifest commit while closing the regression.
Reported via hexpm/hex#1166, where a workaround was added to delete
the stale `.app` from Hex's SCM after each fetch.
Covers the full chain seen in hexpm/hex#1166: a fetchable dep's stale .app is read by `validate_app`, produces an `{:ok, vsn}` that matches the top-level requirement, then a transitive parent with a stricter requirement causes the converger's `req_mismatch` to mark the dep as `:divergedreq` — even though the only thing actually wrong is that `_build` hasn't been recompiled yet. With the loader bypass, `validate_app` returns `:compile` for a fetchable dep whose SCM manifest's stored lock differs from `opts[:lock]`, so the dep never enters the converger with `{:ok, _}` and `req_mismatch` doesn't fire. The previous test exercised the loader short-circuit in isolation; this one exercises the same path through the full converge step and asserts `Mix.Dep.diverged?/1` returns false.
Member
Author
|
The commit that broke the hex test suite: 7fa1914e88 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a fetchable dependency is re-fetched at a new version, the source in
deps/<name>is replaced but the build artifacts under_build/<env>/lib/<name>are not. The dep loader'svalidate_appreads the stale_build/.../<name>.app, sees the previous version string, and either reports:nomatchvsnagainst the new requirement or, more subtly, returns{:ok, old_vsn}which then trips the converger'sreq_mismatchcheck and surfaces as:divergedreq.This used to be guarded by
recently_fetched?, which short-circuitedvalidate_appto:compilewhenever a.fetchmarker (latercompile.fetchthencompile.elixir_scm) indicated the build was behind the source. That guard was removed in "Store the lock in the manifest" in favor ofcheck_manifestcomparing the stored lock against the current lock and marking the dep as:compile. Butcheck_manifestis only reached viacheck_lock, which is guarded byavailable?(dep)— and oncevalidate_apphas produced{:ok, _}, the converger can set:divergedreqfirst, which makes the dep diverged andavailable?false, so the manifest lock check never fires.Restore the short-circuit in
validate_appusing the new signal: if the SCM manifest's stored lock differs fromopts[:lock](or the manifest is missing), treat the dep as:compileand skip validating the stale.app. This preserves the architecture introduced by the lock-in-manifest commit while closing the regression.Found in hexpm/hex#1166.