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

Master regression fixes #2481

Merged
merged 2 commits into from
Sep 17, 2022
Merged

Master regression fixes #2481

merged 2 commits into from
Sep 17, 2022

Conversation

s-ludwig
Copy link
Member

  • Fixes the scanning order of packages, so that add-path packages have precedence over cached packages
  • Fixes compilation on LDC 1.30.0

Why was it decided to drop all compiler support earlier than "ldc-latest" and "dmd-latest" (which doesn't explain why it didn't even compile on ldc-latest)? Having a buffer range of a few compiler versions greatly reduces friction during version transitions, not only in the build environment for dub itself, but also for dependencies of dub that might move slower. And IMHO using the latest shiny features, such as throw expressions in an infrastructure project like this is just utterly unnecessary. I think this really was a bad decision and should be revised, if possible. BTW, compiling with DMD 1.29.0 on Windows still produces backend errors due to some SumType.

Packages registered using add-path need to come before cached packages.
@Geod24
Copy link
Member

Geod24 commented Sep 15, 2022

Hum that was done in 3c1c005 and I agree it wasn't a great idea. I'd be very happy with supporting more versions. The usage of the throw expression is, I think, more accidental than something I wanted absolutely (IOW: I wrote the code, it compiled).

Comment on lines +1144 to +1145
if (this.packagePath !is NativePath.init)
this.scanPackageFolder(this.packagePath, mgr, existing);
Copy link
Member

Choose a reason for hiding this comment

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

The scan order shouldn't matter, the iteration order is done in getPackageIterator, which IIUC is where you want to shift things.

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is that both, search path packages and package path packages end up in the same array (fromPath) and the order within this array matters.

@s-ludwig
Copy link
Member Author

Regarding the compile error, I'm not sure what exactly happens here, it obviously also works for the CI jobs, but not with the Windows version of LDC that I have installed (from the standard distribution ZIP file).

@s-ludwig
Copy link
Member Author

Warning Package maven-dubpackage not found for maven repository at http://localhost:13733/maven/release/dubpackages: Failed to download http://localhost:13733/maven/release/dubpackages/maven-dubpackage/maven-metadata.xml

Not sure what to do about this one, doesn't seem to be related to the changes.

@Geod24
Copy link
Member

Geod24 commented Sep 15, 2022

There's a spurious failure for this: #2345

@s-ludwig
Copy link
Member Author

Okay, all tests have passed after a re-run.

Copy link
Member

@Geod24 Geod24 left a comment

Choose a reason for hiding this comment

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

LGTM but the PackageManager change could use a test to prevent future regression.

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.

2 participants