Fix add-path based development workflow#2526
Merged
Conversation
s-ludwig
force-pushed
the
add_path_workflow
branch
2 times, most recently
from
222d666 to
ff2b2fd
Compare
s-ludwig
force-pushed
the
add_path_workflow
branch
from
ff2b2fd to
17d0987
Compare
Geod24
approved these changes
Nov 7, 2022
Member
Geod24
left a comment
Geod24
left a comment
There was a problem hiding this comment.
It's pretty bad that we need to scan all possible packages on the system for a specific feature, but at least the next time I try it a test will break.
Contributor
|
Does this regress the performance of lookup? |
Member
Author
|
@nordlow There was a functional regression introduced by the performance optimization, not the other way around. Should be back to where it was before now (unfortunately). @Geod24 We should really look into splitting up the add-path scanning and the cache scanning anyway, as both kinds of packages currently share the same array kind of awkwardly. Then it would at least be straight forward to skip the cache scanning part. The add-path part is then basically "pay for what you actually use". |
Geod24
added a commit
to Geod24/dub
that referenced
this pull request
Jun 4, 2024
This was introduced in dlang#2526, however it has been superseded by 1acfe7b . Since the unittest version does not require cleanup and is simpler to run, we can remove the shell version.
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
3 participants
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.
The logic that locally checked out packages added using
add-pathtake precedence and don't follow thestrictmatching rules got broken again due to a recent change. It should be noted that this fix requires a call toPackageManager.refresh(), which will, in order to get the anticipated performance improvement, probably need to be the target for a more fine-grained approach, where only the cache directory is skipped during package scanning instead of everything.