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

Fix 1180: local cache broken #1568

Merged
merged 1 commit into from Dec 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions source/dub/dub.d
Expand Up @@ -168,7 +168,7 @@ class Dub {
else
m_packageSuppliers = getPackageSuppliers(additional_package_suppliers, skip_registry);

m_packageManager = new PackageManager(m_dirs.localRepository, m_dirs.systemSettings);
m_packageManager = new PackageManager(m_rootPath, m_dirs.localRepository, m_dirs.systemSettings);

auto ccps = m_config.customCachePaths;
if (ccps.length)
Expand Down Expand Up @@ -260,7 +260,7 @@ class Dub {
{
init(NativePath());
m_overrideSearchPath = override_path;
m_packageManager = new PackageManager(NativePath(), NativePath(), false);
m_packageManager = new PackageManager(NativePath(), NativePath(), NativePath(), false);
updatePackageSearchPath();
}

Expand Down Expand Up @@ -862,7 +862,7 @@ class Dub {

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we use buildPath for Windows support?

Copy link
Contributor Author

@andre2007 andre2007 Oct 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

case PlacementLocation.user: placement = m_dirs.localRepository ~ "packages/"; break;
case PlacementLocation.system: placement = m_dirs.systemSettings ~ "packages/"; break;
}
Expand Down
2 changes: 1 addition & 1 deletion source/dub/generators/build.d
Expand Up @@ -623,7 +623,7 @@ unittest { // issue #1235 - pass no library files to compiler command line when

auto desc = parseJsonString(`{"name": "test", "targetType": "library", "sourceFiles": ["foo.d", "`~libfile~`"]}`);
auto pack = new Package(desc, NativePath("/tmp/fooproject"));
auto pman = new PackageManager(NativePath("/tmp/foo/"), NativePath("/tmp/foo/"), false);
auto pman = new PackageManager(pack.path, NativePath("/tmp/foo/"), NativePath("/tmp/foo/"), false);
auto prj = new Project(pman, pack);

final static class TestCompiler : GDCCompiler {
Expand Down
20 changes: 17 additions & 3 deletions source/dub/packagemanager.d
Expand Up @@ -39,9 +39,20 @@ class PackageManager {

this(NativePath user_path, NativePath system_path, bool refresh_packages = true)
thewilsonator marked this conversation as resolved.
Show resolved Hide resolved
{
m_repositories.length = LocalPackageType.max+1;
m_repositories[LocalPackageType.user] = Repository(user_path ~ "packages/");
m_repositories[LocalPackageType.system] = Repository(system_path ~ "packages/");
m_repositories = [
Repository(user_path ~ "packages/"),
Repository(system_path ~ "packages/")];

if (refresh_packages) refresh(true);
}

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

if (refresh_packages) refresh(true);
}

Expand Down Expand Up @@ -615,6 +626,7 @@ class PackageManager {
}
scanLocalPackages(LocalPackageType.system);
scanLocalPackages(LocalPackageType.user);
scanLocalPackages(LocalPackageType.package_);

auto old_packages = m_packages;

Expand Down Expand Up @@ -681,6 +693,7 @@ class PackageManager {
}
}
}
loadOverrides(LocalPackageType.package_);
loadOverrides(LocalPackageType.user);
loadOverrides(LocalPackageType.system);
}
Expand Down Expand Up @@ -813,6 +826,7 @@ struct PackageOverride {
}

enum LocalPackageType {
package_,
user,
system
}
Expand Down
20 changes: 20 additions & 0 deletions test/issue1180-local-cache-broken.sh
@@ -0,0 +1,20 @@
#!/usr/bin/env bash
DIR=$(dirname "${BASH_SOURCE[0]}")

. "$DIR"/common.sh

PORT=$(($$ + 1024)) # PID + 1024
"$DUB" remove maven-dubpackage --root="$DIR/issue1180-local-cache-broken" --non-interactive --version=* 2>/dev/null || true

"$DUB" build --single "$DIR"/test_registry.d
"$DIR"/test_registry --folder="$DIR/issue1416-maven-repo-pkg-supplier" --port=$PORT &
PID=$!
sleep 1
trap 'kill $PID 2>/dev/null || true' exit

echo "Trying to download maven-dubpackage (1.0.5)"
"$DUB" upgrade --root="$DIR/issue1180-local-cache-broken" --cache=local --skip-registry=all --registry=mvn+http://localhost:$PORT/maven/release/dubpackages

if ! "$DUB" remove maven-dubpackage --root="$DIR/issue1180-local-cache-broken" --non-interactive --version=1.0.5 2>/dev/null; then
die 'DUB did not install package from maven registry.'
fi
1 change: 1 addition & 0 deletions test/issue1180-local-cache-broken.sh.min_frontend
@@ -0,0 +1 @@
2.076
4 changes: 4 additions & 0 deletions test/issue1180-local-cache-broken/.gitignore
@@ -0,0 +1,4 @@
test
*.o
*.exe
.dub
Empty file.
7 changes: 7 additions & 0 deletions test/issue1180-local-cache-broken/dub.json
@@ -0,0 +1,7 @@
{
"name": "test",
"dependencies": {
"maven-dubpackage": "1.0.5"
}

}
1 change: 1 addition & 0 deletions test/issue1180-local-cache-broken/source/app.d
@@ -0,0 +1 @@
void main(){}
2 changes: 1 addition & 1 deletion test/issue674-concurrent-dub.sh
Expand Up @@ -16,4 +16,4 @@ cd ${TMPDIR} && $DUB fetch --cache=local bloom &
pid2=$!
wait $pid1
wait $pid2
[ -d ${TMPDIR}/bloom* ]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@andre2007 andre2007 Oct 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

[ -d ${TMPDIR}/.dub/packages/bloom* ]