Skip to content

fix: reload upgraded deps' modules before composing their upgrade tasks - #404

Open
pshoukry wants to merge 1 commit into
ash-project:mainfrom
pshoukry:fix-stale-upgrade-task
Open

pshoukry wants to merge 1 commit into
ash-project:mainfrom
pshoukry:fix-stale-upgrade-task

Conversation

@pshoukry

Copy link
Copy Markdown
Contributor

Note: this PR was drafted with an AI assistant (Claude) and opened by
me; the reproduction and test results below were run as shown.

Fixes #403

Condition

mix igniter.upgrade runs the old version's <package>.upgrade task when
the old version of that dep was compiled in the same VM before the upgrade.
This happens with a fresh or stale _build, with or without the
igniter_new archive. With _build already compiled for the old lock, the
new task runs.

Cause

  1. The old dep is compiled and loaded in this VM, by Mix before the task or
    by Mix.Task.run("compile", []) at lib/igniter/upgrades.ex:106.
  2. lib/igniter/util/install.ex:287-288 runs mix deps.get and
    mix deps.compile in a separate OS process. The new .beam files land
    on disk; this VM keeps the old modules.
  3. The in-VM compile at lib/igniter/upgrades.ex:189-193 finds nothing
    stale, so nothing purges them.
  4. Mix.Task.get(task) at lib/igniter/upgrades.ex:415 returns the old
    module.

Fix

After the recompile, Igniter.Upgrades.upgrade/1 unloads every loaded
module whose .beam lives in a changed dep's ebin, so Mix.Task.get/1
loads the new task from disk. It skips igniter and the deps it runs on
(igniter, glob_ex, rewrite, sourceror, spitfire); upgrading those
already raises earlier. That list moves into @igniter_apps, shared with
the check.

It never kills a process. For each module it calls :code.soft_purge/1
and deletes the module only when that returns true; after the delete it
soft-purges again, so a process running the deleted version keeps it.

Limit: when a process still runs a module's old code, the module stays
loaded as it is, so that module keeps the old version for this run. The
upgrade task's own modules run in no process, so they always reload.

Tests

Two new tests in test/igniter/upgrades_test.exs:

  • Loads version 1 of a module, writes version 2's .beam to its ebin,
    calls purge_stale_modules/1, and asserts version 2 runs. It fails
    without the fix.
  • Starts a process running a module's old code, then calls
    purge_stale_modules/1. It asserts the process is alive, the module
    still returns version 1, and the process exits normally. It fails with
    :code.purge/1, which kills the process.

mix test: 1 doctest, 441 tests, 0 failures. mix format --check-formatted,
mix credo --strict and mix dialyzer pass. Elixir 1.18.4, OTP 27.

Reproduction

A git dep with an upgrade task at two versions; 0.2.0 adds a notice.

# upk: a mix project depending on igniter, with Mix.Tasks.Upk.Upgrade.
# v0.1.0 upgrade map: %{}
# v0.2.0 upgrade map:
#   %{"0.2.0" => [fn igniter, _ -> Igniter.add_notice(igniter, "0.2.0") end]}
cd upk && git commit -am v0.1.0

# app/mix.exs deps:
#   [{:upk, git: "file:///path/to/upk", branch: "main"}, {:igniter, "~> 0.8"}]
cd ../app && mix deps.get           # locks upk 0.1.0, no compile

cd ../upk && git commit -am v0.2.0  # publish 0.2.0 on the branch

cd ../app && mix igniter.upgrade upk --yes
# before: the 0.1.0 task runs, no notice
# after:  the 0.2.0 task runs and prints the "0.2.0" notice

Running mix compile before mix igniter.upgrade hides the bug.

Contributor checklist

  • I accept the AI Policy, or AI was not used in the creation of this PR.
  • Bug fixes include regression tests

@zachdaniel

Copy link
Copy Markdown
Contributor

This is pretty dicey. I'm going to have to think about this one 😓

@pshoukry

Copy link
Copy Markdown
Contributor Author

@zachdaniel please let me know if you need me to do any additional validation or help with anything.

This branch has not been deployed

No deployments
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.

igniter.upgrade runs the old version of a package upgrade task after the recompile

2 participants