PackageManager: Store packages hierarchically, by version

[Geod24]

As explained in the changelog, this hierarchy makes much more sense than the current one, and will make it possible to read the version from the directory name.

[Geod24]

Tested locally, will add test, but would prefer to do it with #2603

[s-ludwig]

I don't think the rationale is super convincing, in particular a package GC and path based version inference are possible just fine with the flat approach, too. It also increases the iteration overhead by having to open each directory. On the other hand, the nested version scheme does look cleaner when browsing the folder manually. So unless the iteration slowdown is significant, I wouldn't really mind the change either.

Implementation wise, everything looked fine, not sure about the failed tests.

[Geod24]

in particular a package GC and path based version inference are possible just fine with the flat approach, too.

Unfortunately no, the scheme is currently ambiguous. foo-v1.2.3-v4.5.6 could be a package named foo-v1.2.3 with version v4.5.6 or a package named foo with version v1.2.3-v4.5.6. I figured it would be much easier (and cleaner) to go this way than to add tons of heuristic for a corner case.

It also increases the iteration overhead by having to open each directory.

The biggest overhead is that we have to open those directories at all. The vast majority of invocation are dub build (or its derivatives, dub run / dub test), and in the vast majority of cases, dependencies are already resolved. We shouldn't be iterating over those folders. We have a reduced dependency on this thanks to #2500 and previous PRs, but, out of an abundance of caution, we're still doing it as a fallback for now, however it should go away eventually.

On the other hand, the nested version scheme does look cleaner when browsing the folder manually.

Good point, will add it to the rationale.

[Geod24]

Regarding tests, this is already widely tested as can be seen in the changes. Last thing I'm musing over: Should we just tell people to nuke ~/.dub/packages/ ?

[s-ludwig]

Unfortunately no, the scheme is currently ambiguous.

Package names are not allowed to contain ".", so there is no ambiguity here.

The biggest overhead is that we have to open those directories at all.

Are you sure about that? I'd guess that reading the individual dub.json files is the largest factor, but admittedly I have never measured that.

[Geod24]

Package names are not allowed to contain ".", so there is no ambiguity here.

This is not enforced by dub though, right ? We only get a warning AFAIK. Is it enforced by the registry ?

Are you sure about that? I'd guess that reading the individual dub.json files is the largest factor, but admittedly I have never measured that.

Right, "opening those directories" was an abuse of language. The expensive part is the whole "load all packages known to dub on each command". You're right that if we break that down further, reading the dub.json will likely come on top. But if we load packages lazily, which we can do for most cases of dub build, the whole argument is moot because it only affect some rarely used commands (dub list, dub build on the first resolution). And my guess is that, if we start to infer the packages from the FS, dub list will benefit as well.

[WebFreak001]

what happens with the old folder structure? I'm not seeing any migration code or loading code from the old path names

[Geod24]

I asked Atila what we should do with it. He said something to the tune of "Just do it". So no migration, people can just nuke their package folder and re-download.

[s-ludwig]

This is not enforced by dub though, right ? We only get a warning AFAIK. Is it enforced by the registry ?

Exactly. And since these are just registry packages, those others can't end up there. But I really think we should change the warning to an error here. It shows time again that reserving these characters is very valuable.

[Geod24]

Exactly. And since these are just registry packages, those others can't end up there.

What about repository packages ?

But I really think we should change the warning to an error here. It shows time again that reserving these characters is very valuable.

Agreed.

[WebFreak001]

I asked Atila what we should do with it. He said something to the tune of "Just do it". So no migration, people can just nuke their package folder and re-download.

I don't think this is a good idea and most people won't nuke their package folder, I bet a lot don't even know out of their head where it is.

Migration code should also be super easy to do, so telling people to nuke their dub packages would just be wasting bandwidth and is going to introduce a lot of delay to people building projects again. (especially annoying for people who are coding offline / on the go, trying to work on their projects after a D / dub update the previous night.

[rikkimax]

I don't think this is a good idea and most people won't nuke their package folder, I bet a lot don't even know out of their head where it is.

There is a solution to this. New directory, if old one hasn't been touched in a month, delete it.

[Geod24]

There's quite a few places that rely on the directory ~/.dub/packages/ for periodic cleanup / caching. So not a fan of a new directory. Will look into migration path tomorrow.

[s-ludwig]

What about repository packages ?

I have no idea how they are stored, but if they have their associated version suffixed to the folder, then they will work. If not, they won't work with either scheme.

[s-ludwig]

I have no idea how they are stored,

Should be "stored and updated". I just didn't follow the implementation here and I'm not sure whether that is actually sound.

[s-ludwig]

BTW, I'd be with Atila here, as long as the existing packages are still used and will be purged by a future GC mechanism, having an explicit migration code doesn't seem necessary. Also, people might mix different DUB versions (I sometimes do when accessing the same repository from Windows and from WSL, for example), which would lead to constant conflicts.

[WebFreak001]

hm you are right, that's not a thing I've considered.

Also serve-d is gonna break horribly with auto-complete and build linting because it bundles its own dub version and will differ from the system dub version.

So I guess it might just make more sense to have both in parallel and some time in the future think about migrating to only the new scheme.

[Geod24]

So... Can we merge this ?