fix: seed port allocations for every command, not just up and tasks run - #3063
Open
aostanin wants to merge 2 commits into
Open
fix: seed port allocations for every command, not just up and tasks run#3063aostanin wants to merge 2 commits into
aostanin wants to merge 2 commits into
Conversation
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
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.
Problem
processes.<name>.ports.<port>.valueresolves to the declared base port in everycommand except
devenv upanddevenv tasks run, even while a process manager isrunning with a different allocation. Env vars derived from it are wrong too.
Run it in two clones of the project. The first takes the base port:
The second allocates around it, but its shell reports the first clone's port:
Root cause
reserve_running_ports()seeds the allocator from the running native manager,but is only called from
up()andtasks_run().shell(and thereforedevenv hook, which spawnsdevenv shell) anddirenv-exportnever seed, soallocatePortis not registered and.valuefalls back toallocate.This is a regression of the #2710 fix rather than a gap in it.
88ac631seeded inside
assemble(), which every evaluating command went through;d0f9c588("split apart and remove assemble") kept the call only in
up()andtasks_run(). The example above reports the allocated port at commit6792fc1eand the base port at commit
d0f9c588,so that is the commit where it stopped working.
Fix
Call
reserve_running_ports()once afterDevenv::newinrun_backend, beforeanything evaluates. Best-effort: it only seeds when a live-PID manager answers,
so the gating added in #3024 still applies. The socket query gets a 2s timeout,
since it now runs before every command and a socket that accepts but never
answers must not hang the CLI.
The existing calls in
up()andtasks_run()are left in place — they areredundant for the CLI but still cover direct use of those public methods.
Seeding still requires a live-PID manager, so a command run during a manager's
startup window — between its processes binding their ports and the PID file
being written after the readiness probes — resolves the base port as before.
tests/process-port-allocation-two-reposwaits for the PID file for thatreason. Closing that window is a separate change.
Validation
devenv-run-tests run --only process-port-allocation-two-repos tests, extendedwith a
devenv shellassertion in the second project: fails on v2.2.1, passeswith the fix
devenv-run-tests run --only 'process-*' --only 'tasks-*' tests: 18 passedx86_64-linuxandaarch64-darwin:devenv shell,direnv-exportanddevenv processes listagree on the allocated port