Fix Mix.Dep.Lock.write/2 ignoring :file option when comparing changes

[jisung-02]

Mix.Dep.Lock.write/2 accepts a :file option to write to a custom lockfile path. However, when checking whether the lock has changed, it called read/0 without arguments, comparing the new lock against the default mix.lock rather than the target file.

For example, given:

# mix.lock contains:    %{foo: {:hex, :foo, "0.1.0"}}
# vendor.lock contains: %{bar: {:hex, :bar, "0.1.0"}}

Mix.Dep.Lock.write(
  %{bar: {:hex, :bar, "0.1.0"}},
  file: "vendor.lock",
  check_locked: true
)

vendor.lock is already up to date, so this should be a no-op. Instead, the new map is compared against mix.lock, the values differ, and Mix.Error is raised:

** (Mix.Error) Your vendor.lock is out of date and must be updated without the --check-locked flag

Without check_locked, the same mismatch causes a spurious rewrite of the up-to-date file and an unnecessary Mix.Task.run("will_recompile").

The fix passes the resolved lockfile path to read/1 so the comparison and the write target the same file. A regression test for the check_locked: true case is added.

[josevalim]

Hi @jisung-02, can you please explain when this is a bug? My understanding is that the :file option is only used when comparing across lock files in Mix.install, so the fact the lockfile is not reused is intentional?

[jisung-02]

I looked into this again, and I think you are right.

I opened this PR because, while reading Mix.Dep.Lock.write/2, the internal consistency looked questionable to me. The function resolves the target lockfile from opts[:file], but still uses read/0 when checking whether the lock has changed.

I also created a local reproduction by calling the internal module Mix.Dep.Lock.write/2 directly. In that case, when :file points to a file other than the default mix.lock, the comparison is still performed against mix.lock.

However, after checking the actual callers again, it looks like this does not affect the supported public behavior. So this PR is probably better described as an internal consistency cleanup rather than a bug fix.

After checking again based on your explanation, I’m fine closing this PR if you don’t think this internal cleanup is necessary.