[Spike] Multiple version mode for bootstrap commands #909
Conversation
This commit attempts to implement the multiple version mode for bootstrap and is a result of investigation as to what it might look like. This is NOT the final implementation for the feature but can be a good starting point to discuss how we can implement this Hence this PR is in Draft state. Signed-off-by: Rohan Devasthale <rdevasth@redhat.com>
| # | ||
| # six>=1.16.0 should match all versions from 1.16.0 onwards | ||
| # We use a narrow range to limit how many versions we build. | ||
| PACKAGE_SPEC="six>=1.16.0" |
There was a problem hiding this comment.
We should probably go ahead and cap the range here so new releases don't break the build.
| echo "FAIL: Expected at least 1 six wheel file, found $SIX_WHEEL_COUNT" 1>&2 | ||
| pass=false | ||
| else | ||
| echo "PASS: six versions built" |
There was a problem hiding this comment.
If we cap the range, we can predict exactly how many wheels we should get.
| --sdists-repo="$OUTDIR/sdists-repo" \ | ||
| --wheels-repo="$OUTDIR/wheels-repo-3" \ | ||
| --work-dir="$OUTDIR/work-dir" \ | ||
| bootstrap --all-versions "six==1.16.0" || true # May fail due to constraints conflict |
There was a problem hiding this comment.
This test is confusing. Why would we not want a constraint mismatch to give us an error in this case?
| help=( | ||
| "Build all versions of packages that match the requirements, not just the " | ||
| "newest. Versions already in the cache server are skipped automatically. " | ||
| "Use with --skip-constraints since multiple versions cannot be unified." |
There was a problem hiding this comment.
Constraints can have ranges, too. So we don't want to skip them. We always want them applied.
| if all_versions: | ||
| # In all-versions mode, resolve all matching versions for each | ||
| # top-level requirement. This returns a list of (url, version) pairs. | ||
| results = bt.resolve_and_add_top_level_all_versions(req) |
There was a problem hiding this comment.
What do you think about making all_versions a flag to resolve_and_add_top_level and always have it return a sequence of results?
|
|
||
| # Add to dependency graph | ||
| logger.info("including %s==%s for build", req.name, version) | ||
| self.ctx.dependency_graph.add_dependency( |
There was a problem hiding this comment.
I'm surprised to see logic modifying the graph here when we add something to the list of results to return. Doesn't the bootstrap() function handle modifying the graph as it processes unbuild packages?
| ) | ||
|
|
||
| # Verify the build tag matches our expected tag | ||
| from urllib.parse import urlparse |
|
|
||
| return results | ||
|
|
||
| def _version_exists_in_cache( |
There was a problem hiding this comment.
I suspect this can be replaced with a call to self._find_cached_wheel?
| # source URL from the resolved requirements cache or resolve again. | ||
| try: | ||
| # Create a pinned requirement to get the source URL | ||
| pinned_req = Requirement(f"{req.name}=={resolved_version}") |
There was a problem hiding this comment.
Does req.name includes the extras specifiers, like we would have in vllm[cuda]?
I suspect you're going to want to create a function for taking a requirement and version and giving back a requirement for exactly that version. Putting it in its own function will make it easy to create all of the test cases in a unit test.
| ) | ||
| return | ||
| else: | ||
| # Normal resolution: find the best matching version |
There was a problem hiding this comment.
Here we again want a list of results, not just the single result. That way we build all of the versions of all of the dependencies, too, not just the top level packages. In fact, I think if we do the expansion here, instead of in the command code, it will make all of the logic simpler.
For example, if at this point we find all matches for a requirement and get a list with more than one item, then we can loop over that list calling bootstrap recursively and then return. If we get a list with 1 item, then we do the bootstrapping work as we're doing it today.
That will mean we only have to look for the all_versions flag in one place, instead of everywhere we process the dependencies.
2 participants
This commit attempts to implement the multiple
version mode for bootstrap and is a result of
investigation as to what it might look like.
This is NOT the final implementation for the feature but can be a good
starting point to discuss how we can implement this
Hence this PR is in Draft state.
Co-Authored-By: [Claude Sonnet 4.5]<noreply@anthropic.com>