Skip to content

Improve the diagnostics that are emitted when a package dependency specifies a non-existent branch or commit #2925

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

Merged
merged 1 commit into from
Sep 14, 2020

Conversation

abertelrud
Copy link
Contributor

This also sets the code up to make it easier to do other similar heuristics as needed, and as a bonus, adds one that is related to the master -> main renaming (suggesting main if master is requested and doesn't exist, but main does). There isn't yet any support for fix-its in SwiftPM, but this would be a case for such a facility once it is supported.

Note that tags go through different processing, since they are the inputs to the semantic versioning and missing tags are reported differently. There is no way in SwiftPM to define an explicit dependency on a non-semver tag, which is why this code only needs to handle branches and commits.

The error message does not list all the branches, since there can be many, but that could be another future improvement (or at least to list all the branches whose names are similar to what was requested).

rdar://34099187

…ecifies a non-existent branch or commit

This also sets the code up to make it easier to do other similar heuristics as needed, and as a bonus, adds one that is related to the `master` -> `main` renaming (suggesting `main` if `master` is requested and doesn't exist, but `main` does).  There isn't yet any support for fix-its in SwiftPM, but this would be a case for such a facility once it is supported.

Note that tags go through different processing, since they are the inputs to the semantic versioning and missing tags are reported differently.  There is no way in SwiftPM to define an explicit dependency on a non-semver tag, which is why this code only needs to handle branches and commits.

The error message does not list all the branches, since there can be many, but that could be another future improvement (or at least to list all the branches whose names are similar to what was requested).

rdar://34099187
@abertelrud
Copy link
Contributor Author

@swift-ci please smoke test

@abertelrud abertelrud merged commit 75b8352 into swiftlang:master Sep 14, 2020
@compnerd
Copy link
Member

Hmm, I seem to be getting an error on Windows after this change:

cmd.exe /C "cd . && S:\22\toolchain-windows-x64\Library\Developer\Toolchains\unknown-Asserts-development.xctoolchain\usr\bin\swiftc.exe -target x86_64-unknown-windows-msvc -output-file-map Sources\PackageGraph\CMakeFiles\PackageGraph.dir\output-file-map.json -incremental -j 48 -emit-library -o bin\PackageGraph.dll -module-name PackageGraph -module-link-name PackageGraph -emit-module -emit-module-path swift\PackageGraph.swiftmodule -emit-dependencies -DPackageGraph_EXPORTS -resource-dir "S:\22/sdk-windows-x64/Library/Developer/Platforms/Windows.platform/Developer/SDKs/Windows.sdk/usr/lib/swift" -LS:\22/sdk-windows-x64/Library/Developer/Platforms/Windows.platform/Developer/SDKs/Windows.sdk/usr/lib/swift/windows -O -swift-version 5 -libc MD   -Xcc -D_CRT_SECURE_NO_WARNINGS -I swift -I S:\22\b\swift-tools-support-core\swift -I S:\22\s\swift-tools-support-core\Sources\TSCclibc\include -I S:\22\b\swift-llbuild\products\llbuildSwift -I S:\22\s\swift-llbuild\products\libllbuild\include S:\22\s\swift-package-manager\Sources\PackageGraph\CheckoutState.swift S:\22\s\swift-package-manager\Sources\PackageGraph\DependencyResolver.swift S:\22\s\swift-package-manager\Sources\PackageGraph\PackageGraph.swift S:\22\s\swift-package-manager\Sources\PackageGraph\PackageGraphLoader.swift S:\22\s\swift-package-manager\Sources\PackageGraph\PackageGraphRoot.swift S:\22\s\swift-package-manager\Sources\PackageGraph\PinsStore.swift S:\22\s\swift-package-manager\Sources\PackageGraph\Pubgrub.swift S:\22\s\swift-package-manager\Sources\PackageGraph\RawPackageConstraints.swift S:\22\s\swift-package-manager\Sources\PackageGraph\RepositoryPackageContainerProvider.swift S:\22\s\swift-package-manager\Sources\PackageGraph\VersionSetSpecifier.swift    -Xlinker -implib:lib\PackageGraph.lib -L S:/22/b/swift-package-manager/lib  -L S:/22/b/swift-package-manager/lib  -L S:/22/b/swift-package-manager/lib  -L S:/22/b/swift-package-manager/lib  -L S:/22/b/swift-llbuild/lib  -L S:/22/b/swift-llbuild/lib  -L S:/22/b/swift-tools-support-core/lib  -L S:/22/b/swift-tools-support-core/lib  -L S:/22/b/swift-tools-support-core/lib  -L S:/22/b/swift-tools-support-core/lib lib\PackageLoading.lib  lib\PackageModel.lib  lib\SourceControl.lib  lib\SPMLLBuild.lib  S:\22\b\swift-llbuild\lib\llbuildSwift.lib  S:\22\b\swift-llbuild\lib\llbuild.lib  S:\22\b\swift-tools-support-core\lib\TSCUtility.lib  S:\22\b\swift-tools-support-core\lib\TSCBasic.lib  S:\22\b\swift-tools-support-core\lib\TSCLibc.lib  S:\22\b\swift-tools-support-core\lib\TSCclibc.lib  -lFoundation.lib  -lFoundationNetworking.lib && cd ."
S:\22\s\swift-package-manager\Sources\PackageGraph\RepositoryPackageContainerProvider.swift:399:91: error: value of type 'ProcessResult.Error' has no member 'description'
            if let gitInvocationError = error as? ProcessResult.Error, gitInvocationError.description.contains("Needed a single revision") {
                                                                       ~~~~~~~~~~~~~~~~~~ ^~~~~~~~~~~

compnerd added a commit to compnerd/swift-tools-support-core that referenced this pull request Sep 14, 2020
…ger#2925

The `ProcessResult.Error` conformance to `CustomStringConvertible` was
ignored on the Windows target accidentally.  Adjust this to repair the
build.
@compnerd
Copy link
Member

swiftlang/swift-tools-support-core#124 should repair the build

@abertelrud
Copy link
Contributor Author

Thanks @compnerd and apologies. It looks as thought the #endif in TSC was misplaced all along, I think? Can't wait to get Windows CI runs, since I don't have a Windows box to test changes with.

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