Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PackageManager must know placement dir #2155

Closed

Conversation

denizzzka
Copy link
Contributor

@denizzzka denizzzka commented Jul 21, 2021

upd: For now if you try to replace PackageManager arguments (representing repositories dirs) you will catch inconsistency between provided dirs where dub will search packages and dirs where dub stores fetched packages.

This PR:

  • Removes duplicate definition of paths to local repositories
  • Adds possibility for user defined PackageManager (PR what opens this ability is coming soon)

@denizzzka denizzzka changed the title PackageManager knows placement dir PackageManager must know placement dir Jul 21, 2021
@rikkimax
Copy link
Contributor

It might be a good idea to add a second package manager instance to verify that everything has been caught.

e.g. regex match packages names into a custom directory.

@denizzzka
Copy link
Contributor Author

denizzzka commented Jul 21, 2021

@rikkimax can you explain more detail your idea, what problem can be solved by second PackageManager?

@rikkimax
Copy link
Contributor

@rikkimax can you explain more detail your idea, what problem can be solved by second PackageManager?

To verify that this change works correctly both today and will continue to work in the future.

There could be some hidden assumptions in some place that can only be found by doing this.

@denizzzka
Copy link
Contributor Author

denizzzka commented Jul 21, 2021

There could be some hidden assumptions in some place that can only be found by doing this.

For now in ~master if you try to replace PackageManager arguments (representing repositories dirs) you will catch inconsistency between provided dirs where dub will search packages and dirs where dub stores fetched packages.

In this PR possibility of this is excluded - it totally removes this paths from Dub.

So, I think, it isn't necessary. Or conside this PR as bugfix :-)

@denizzzka denizzzka force-pushed the simplify_placement_paths_purged branch from 71a8d29 to e02ef2a Compare July 21, 2021 14:03
@denizzzka denizzzka marked this pull request as ready for review July 21, 2021 16:59
@Geod24
Copy link
Member

Geod24 commented Sep 11, 2022

So, after countless refactorings of the PackageManager, I don't think it should know the default PlacementLocation.
Instead, the API offers 2 possibilities:

  • Act on all possible PlacementLocation (usually reads);
  • Act on a specific PlacementLocation (usually write);

The Dub class is responsible for choosing the PlacementLocation and storing the default. Feel free to re-open if you disagree and have a use case I am not seeing.

@Geod24 Geod24 closed this Sep 11, 2022
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.

3 participants