dub
dub copied to clipboard
dlang
rethink of caching and user/system repos
I recognise this looks like an absolute monster diff, so I'll do my best to put in a lot of comments in here to explain motivations. Quite a lot of the diff is trivial, so please don't be put off by the raw size.
If you like to break things down by commit: https://github.com/dlang/dub/commit/06a40361386cfd746aa152687d535ec50b00ead6, https://github.com/dlang/dub/commit/13d6ec41849b7680076bd5c0f6cb1e10413b23f6 and https://github.com/dlang/dub/commit/82d25b62a151021a2916f4fc9bf36e72046de324 are the places to look, the test suite should pass for each of them individually. Not much to be gained by looking at the other two in isolation.
The changes are also broadly contained within each file: packagemanager.d first, then dub.d, then commandline.d. Alternatively, the opposite order might make the "why" clearer.
The point:
Adding support for custom directories for packages and local registration files. Currently there is --cache for anything fetched and the --system flag for registration commands, but there is no way to specify a custom directory for either fetching or registering.
A common use-case (that I currently have) is a large project with many dub packages in it, which much be registered with dub, but don't have any purpose outside that project. It also allows any dependencies to be easily packaged inside a project in order to allow building an application without an internet connection, by just running dub upgrade --root=$APP_PROJECT_DIR --defaultRepoPath=$PROJECT_ROOT/deps/ on the dev machine, then dub build --root=$APP_PROJECT_DIR --defaultRepoPath=$PROJECT_ROOT/deps/ --skip-registry=all on the target machine.
tests still to come, so there are likely to be bugs that need fixing.
Coverage decreased (-0.2%) to 61.263% when pulling 82d25b62a151021a2916f4fc9bf36e72046de324 on John-Colvin:refactor into 6601fc83a509b8af005df5ac0f75ff7b56a50d50 on dlang:master.
Coverage decreased (-0.2%) to 61.263% when pulling 276c41740d70f5fbf3ed8488935dc8a2fe95379b on John-Colvin:refactor into 6601fc83a509b8af005df5ac0f75ff7b56a50d50 on dlang:master.
Just two generic comments, because I don't have the time for doing a review, yet: There should be no breaking API changes (just in case), and I have the feeling that the diff might be larger than it needs to be in order to implement the functionality. I'll have to take a closer look to be certain, but I think that this should be possible by reusing existing functionality.
Coverage decreased (-0.09%) to 61.375% when pulling d35f5067e53de862ab55b2c47fe52efb47fe7c9f on John-Colvin:refactor into 6601fc83a509b8af005df5ac0f75ff7b56a50d50 on dlang:master.
Coverage decreased (-0.09%) to 61.375% when pulling 28ca71f435a9b5bc4f103c8635ce78ca8fd82bef on John-Colvin:refactor into 6601fc83a509b8af005df5ac0f75ff7b56a50d50 on dlang:master.
There should be no breaking API changes
There are (almost) none. I think the only one is that defaultPlacementLocation is no longer readable, only writeable, because it would no longer make sense (the location could be something that isn't in enum PlacementLocation { ... } (See the inline github comment for an alternative). Every other change is backwards compatible, but the old versions are marked deprecated. Users inheriting from the various classes shouldn't see breakage as I've been quite careful to keep methods virtual (i.e. no templates, even when they would have been nice).
the diff might be larger than it needs to be in order to implement the functionality
It is larger than strictly necessary for the command-line flag. 1) repository locations are now controllable from the API in a straightforward manner. 2) There's a fair amount of refactoring and general improvement bits and pieces.
Coverage decreased (-0.09%) to 61.375% when pulling 369707a8c9853d6b96dbaab04969dfbbcf81fcc1 on John-Colvin:refactor into 6601fc83a509b8af005df5ac0f75ff7b56a50d50 on dlang:master.
@John-Colvin : I see a lot of overlap between what you were attempting to do here and the current state of the PackageManager. Namely, things were re-centered around Location (former Repository) and the PackageManager is now a hub to access Locations.
I see that the main change was to index things by Path, which, as you described it, is scary. The way one can support unlimited Path right now is by adding search paths to a specific location. Do you think the current state of master covers what you tried to do (albeit differently), or is there something still missing ?
