Fix 1180: local cache broken #1568

andre2007 · 2018-09-24T15:44:46Z

If you use --cache=local

e.g.

dub init vibetest -t vibe.d
cd vibetest
dub build --cache=local

the dependencies are stored in the package folder but ignored by dub. That was the reason why for
the example the error message "Non-optional dependency vibe-d of vibetest not found in dependency tree!?." is raised.

This pull request propose to store the packages for --cache=local in folder <dub-package-folder>/.dub/packages instead of <dub-package-folder>/

This new location is also added the the package folders, Dub is aware of.

Benefits

no conflicts with sub packages
dependencies are cached in a structured manner and the noise in the dub package folder is removed
as .dub folder is already in gitignore, the cached dependencies are also not tracked by git
consistent with user and system cache

dlang-bot · 2018-09-24T15:44:48Z

Thanks for your pull request and interest in making D better, @andre2007! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
I have provided a detailed rationale explaining my changes
New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

andre2007 · 2018-09-25T20:55:55Z

Also a questions/proposal: should the dependencies which are retrieved with cache=local be stored in a sub folder "packages" of the dub package folder?

At the moment they are stored directly in dub package folder.

By storing them in a sub folder the developer has a better overview and it is more consistent with the user and system cache.

source/dub/packagemanager.d

WebFreak001 · 2018-09-29T18:21:13Z

test/issue1180-local-cache-broken/dub.json

+{
+	"name": "test",
+	"dependencies": {
+		"local-cache-lib": "~master"


I don't think this is a good idea because it could add unwanted overrides (in either direction) if the user uses this to load their submodules as dependency because it could conflict with a package on the dub registry. Also not specifying path is making this really hard to parse for existing tools.

It is hard to answer here, because I do not know whether --cache=local is fully implemented and works as designed or a half implemented feature. There is no documentation.

Yes, as the depencenies(fetched with cache local) stored in the package folder, this could conflict with sub modules. My idea is to enhance the cache=local functionality and store the depencenies in a subfolder "packages".

But maybe there is a reason why cache=local works as it works. Is the idea that the developer adds the path by hand to dub json?

WebFreak001

Don't think allowing the package folders directly relative to the project without specifying path is a good idea.

andre2007 · 2018-09-29T18:47:22Z

I understand your last answer as response to my proposal?
In my opinion the developers expects that cache=local works similar to the user/system cache. The dependencies are retrieved, stored and automatically found by dub. If the intention is that the developers need to add these depencies by hand to dub.json maybe we could document it.

WebFreak001 · 2018-09-29T18:51:32Z

Well the cache is not something supposed to be added or edited manually (for permanent solutions) which this PR would imply and allow people to do.

Reasons are that for example if vibe would start to use this as way to depend on some local package then anyone could just create a malicious package on the dub registry with the same name as the now depended package, running some prebuild command messing with your local machine.

Also the other way around if the local packages are higher in precedence it could happen that you add a subpackage with the same name as a dub package you would like to depend on (for example you make a opengl subpackage for your package)

andre2007 · 2018-09-29T19:14:07Z

I need some hours to reason about. For the moment I am not sure what steps needs to be done.

store packages in $dub_package_folder (current implementation)
store packages in $dub_package_folder/packages
store packages in $dub_package_folder/.dub/packages
remove cache=local
other solution?

andre2007 · 2018-10-05T18:29:57Z

I implemented option 3. Please see pr description for the benefits. Test is adapted.

andre2007 · 2018-10-13T13:25:44Z

@WebFreak001 Could you have a second look?
Thanks.

andre2007 · 2018-10-19T11:19:59Z

@s-ludwig @wilzbach
Could you have a look?

wilzbach

Not sure about this, the cache only makes sense on a global level, no?

wilzbach · 2018-10-31T09:10:13Z

source/dub/dub.d

@@ -804,7 +804,7 @@ class Dub {

 		NativePath placement;
 		final switch (location) {
-			case PlacementLocation.local: placement = m_rootPath; break;
+			case PlacementLocation.local: placement = m_rootPath ~ ".dub/packages/"; break;


Shouldn't we use buildPath for Windows support?

It is working fine on windows, as placement is of type NativePath.
Should I still change coding to buildPath?

wilzbach · 2018-10-31T09:10:48Z

source/dub/packagemanager.d

+    this(NativePath package_path, NativePath user_path, NativePath system_path, bool refresh_packages = true)
+	{
+		m_repositories = [
+			Repository(package_path~ ".dub/packages/"),


It is working fine on windows as Repository constructor also uses NativePath.

wilzbach · 2018-10-31T09:11:59Z

test/issue674-concurrent-dub.sh

@@ -15,5 +15,4 @@ sleep 0.5
 cd ${TMPDIR} && $DUB fetch --cache=local bloom &
 pid2=$!
 wait $pid1
-wait $pid2
-[ -d ${TMPDIR}/bloom* ]


Isn't this nullifying the test? Why was this necessary?

Fixed, changed to [ -d ${TMPDIR}/.dub/packages/bloom* ]

andre2007 · 2018-10-31T13:40:50Z

The local cache makes most sense for me on CI servers like Jenkins.
For my projects I want always an empty package list. The easiest way is to use cache=local

Also after the build, I need all of my project files and all dependencies source code files for further processing. For example I want to execute WhiteSource on them which is a tool for IP scanning. This is also quite easy with cache=local.

wilzbach · 2018-10-31T20:00:11Z

I always hacked around this with HOME=$(pwd) dub..., but yeah I can now see from what point you are coming from. Thanks!

wilzbach · 2018-10-31T20:01:33Z

Though I would love to hear a third option on this before merging, @s-ludwig or @MartinNowak maybe?

Geod24 · 2019-12-20T10:05:36Z

Another reason the user would want to have those packages locally is for interaction with tools, such as IDE, or simply docker, as docker build calling dub build would ALWAYS re-download the packages by default.

@andre2007 : Can you rebase on master ?

andre2007 · 2019-12-20T21:15:57Z

@Geod24 it is rebased now.

Geod24

Tested, LGTM

Comments have been addressed, packages are now stored in .dub/packages/

andre2007 mentioned this pull request Sep 24, 2018

cache local ist broken #1180

Closed

WebFreak001 reviewed Sep 29, 2018

View reviewed changes

source/dub/packagemanager.d Show resolved Hide resolved

WebFreak001 reviewed Sep 29, 2018

View reviewed changes

WebFreak001 previously requested changes Sep 29, 2018

View reviewed changes

This was referenced Oct 21, 2018

build command ignores '--cache' #1556

Closed

Allow fetching packages to .dub/packages #319

Closed

wilzbach reviewed Oct 31, 2018

View reviewed changes

andre2007 mentioned this pull request May 8, 2019

Add ability to place packages into a specific directory #1699

Closed

Fix local cache

088a7a7

Geod24 approved these changes Dec 21, 2019

View reviewed changes

Geod24 merged commit 7336602 into dlang:master Dec 21, 2019

andre2007 mentioned this pull request Mar 27, 2020

dub fetch documentation example doesn't work #1908

Open

andre2007 deleted the fix_local_cache branch October 16, 2020 18:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix 1180: local cache broken #1568

Fix 1180: local cache broken #1568

andre2007 commented Sep 24, 2018 •

edited

dlang-bot commented Sep 24, 2018 •

edited

andre2007 commented Sep 25, 2018 •

edited

WebFreak001 Sep 29, 2018

andre2007 Sep 29, 2018

WebFreak001 left a comment

andre2007 commented Sep 29, 2018

WebFreak001 commented Sep 29, 2018

andre2007 commented Sep 29, 2018

andre2007 commented Oct 5, 2018

andre2007 commented Oct 13, 2018 •

edited

andre2007 commented Oct 19, 2018

wilzbach left a comment

wilzbach Oct 31, 2018

andre2007 Oct 31, 2018 •

edited

wilzbach Oct 31, 2018

andre2007 Oct 31, 2018 •

edited

wilzbach Oct 31, 2018

andre2007 Oct 31, 2018 •

edited

andre2007 commented Oct 31, 2018

wilzbach commented Oct 31, 2018

wilzbach commented Oct 31, 2018

Geod24 commented Dec 20, 2019

andre2007 commented Dec 20, 2019

Geod24 left a comment

Fix 1180: local cache broken #1568

Fix 1180: local cache broken #1568

Conversation

andre2007 commented Sep 24, 2018 • edited

dlang-bot commented Sep 24, 2018 • edited

andre2007 commented Sep 25, 2018 • edited

WebFreak001 Sep 29, 2018

Choose a reason for hiding this comment

andre2007 Sep 29, 2018

Choose a reason for hiding this comment

WebFreak001 left a comment

Choose a reason for hiding this comment

andre2007 commented Sep 29, 2018

WebFreak001 commented Sep 29, 2018

andre2007 commented Sep 29, 2018

andre2007 commented Oct 5, 2018

andre2007 commented Oct 13, 2018 • edited

andre2007 commented Oct 19, 2018

wilzbach left a comment

Choose a reason for hiding this comment

wilzbach Oct 31, 2018

Choose a reason for hiding this comment

andre2007 Oct 31, 2018 • edited

Choose a reason for hiding this comment

wilzbach Oct 31, 2018

Choose a reason for hiding this comment

andre2007 Oct 31, 2018 • edited

Choose a reason for hiding this comment

wilzbach Oct 31, 2018

Choose a reason for hiding this comment

andre2007 Oct 31, 2018 • edited

Choose a reason for hiding this comment

andre2007 commented Oct 31, 2018

wilzbach commented Oct 31, 2018

wilzbach commented Oct 31, 2018

Geod24 commented Dec 20, 2019

andre2007 commented Dec 20, 2019

Geod24 left a comment

Choose a reason for hiding this comment

andre2007 commented Sep 24, 2018 •

edited

dlang-bot commented Sep 24, 2018 •

edited

andre2007 commented Sep 25, 2018 •

edited

andre2007 commented Oct 13, 2018 •

edited

andre2007 Oct 31, 2018 •

edited

andre2007 Oct 31, 2018 •

edited

andre2007 Oct 31, 2018 •

edited