Skip to content

Apply mirrors to root dependencies #6765

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 12, 2023
Merged

Apply mirrors to root dependencies #6765

merged 1 commit into from
Sep 12, 2023

Conversation

neonichu
Copy link
Contributor

@neonichu neonichu commented Aug 1, 2023

We weren't applying mirros to root dependencies so far.

rdar://110869499

@neonichu neonichu self-assigned this Aug 1, 2023
neonichu
neonichu commented Aug 1, 2023
View reviewed changes
@neonichu
Copy link
Contributor Author

neonichu commented Aug 1, 2023

@swift-ci please smoke test

@neonichu
Copy link
Contributor Author

neonichu commented Aug 1, 2023

Not sure yet how to test this.

@MaxDesiatov
Copy link
Contributor

MaxDesiatov commented Aug 1, 2023 •
edited
Loading

cc @weissi this may be the reason we had to apply workarounds previously for mirroring to fully work

@weissi
Copy link
Contributor

weissi commented Aug 1, 2023

cc @weissi this may be the reason we had to apply workarounds previously for mirroring to fully work

The issue I had was repos that use git submodules because the mirrors don't apply there :|

@neonichu
Copy link
Contributor Author

neonichu commented Aug 1, 2023

git submodules

😱

@tomerd
Copy link
Contributor

tomerd commented Aug 1, 2023

The issue I had was repos that use git submodules because the mirrors don't apply there :|

I actually looked into that a while ago and there is a way to instruct git to do the right thing, I cant remember the details now...

@MaxDesiatov
Copy link
Contributor

I actually looked into that a while ago and there is a way to instruct git to do the right thing, I cant remember the details now...

Yes, that's how we work around it, by generating a .gitconfig file that points to mirrors.

@neonichu neonichu force-pushed the mirrors-for-root-dependencies branch from df4912d to dafd9d3 Compare August 21, 2023 21:32
@neonichu
Copy link
Contributor Author

@swift-ci please smoke test

@neonichu
Copy link
Contributor Author

@swift-ci please test windows

@neonichu neonichu force-pushed the mirrors-for-root-dependencies branch from dafd9d3 to d31a027 Compare September 5, 2023 22:44
@neonichu
Copy link
Contributor Author

neonichu commented Sep 5, 2023

@swift-ci please test

neonichu
neonichu commented Sep 5, 2023
View reviewed changes
tomerd
tomerd reviewed Sep 5, 2023
View reviewed changes
tomerd
tomerd reviewed Sep 5, 2023
View reviewed changes
tomerd
tomerd reviewed Sep 5, 2023
View reviewed changes
tomerd
tomerd reviewed Sep 5, 2023
View reviewed changes
@swiftlang swiftlang deleted a comment from tomerd Sep 5, 2023
@neonichu
Copy link
Contributor Author

neonichu commented Sep 5, 2023

Windows build failure looks pretty weird

SKSwiftPMWorkspace.lib(SwiftPMWorkspace.swift.obj) : error LNK2019: unresolved external symbol __imp_$s12SPMBuildCore15BuildParametersV8dataPath13configuration9toolchain10hostTriple06targetJ05flags20pkgConfigDirectories13architectures7workers27shouldLinkStaticSwiftStdlib0R21EnableManifestCaching31canRenameEntrypointFunctionName0R29CreateDylibForDynamicProducts10sanitizers18enableCodeCoverage14indexStoreMode31enableParseableModuleInterfaces013useIntegratedU6Driver017useExplicitModuleC007isXcodeC13SystemEnabled17enableTestability18forceTestDiscovery014testEntryPointF042explicitTargetDependencyImportCheckingMode15linkerDeadStrip15colorizedOutput13verboseOutput24linkTimeOptimizationMode15debugInfoFormat0R12SkipBuilding22experimentalTestOutputAC6Basics08AbsoluteF0V_12PackageModel0C13ConfigurationOA10_9Toolchain_pA7_0J0VSgA16_A10_0C5FlagsVSayA9_GSaySSGSgs6UInt32VS4bA10_17EnabledSanitizersVSbAC14IndexStoreModeOS5bSgSbA9_SgAC34TargetDependencyImportCheckingModeOS3bAC0S20TimeOptimizationModeOSgAC15DebugInfoFormatOS2btKcfC referenced in function $s18SKSwiftPMWorkspace05SwiftB0C13workspacePath17toolchainRegistry10fileSystem10buildSetup27reloadPackageStatusCallbackAC8TSCBasic08AbsoluteE0V_6SKCore09ToolchainG0CAI04FileI0_pAL05BuildK0VyAA06ReloadmN0OctKcfc
bin\sourcekit-lsp.exe : fatal error LNK1120: 1 unresolved externals
clang: error: linker command failed with exit code 1120 (use -v to see invocation)
<unknown>:0: error: link command failed with exit code 1120 (use -v to see invocation)
ninja: build stopped: subcommand failed.

@MaxDesiatov
Copy link
Contributor

@swift-ci test

MaxDesiatov
MaxDesiatov reviewed Sep 7, 2023
View reviewed changes
MaxDesiatov
MaxDesiatov reviewed Sep 7, 2023
View reviewed changes
Sources/PackageModel/IdentityResolver.swift Outdated
@@ -20,6 +21,10 @@ public protocol IdentityResolver {
func resolveIdentity(for path: AbsolutePath) throws -> PackageIdentity
func mappedLocation(for location: String) -> String
func mappedIdentity(for identity: PackageIdentity) throws -> PackageIdentity

func mappedDependency(for dependency: PackageDependency, fileSystem: FileSystem) throws -> PackageDependency
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: missing doc comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here, I don't think I can describe the logic here.

MaxDesiatov
MaxDesiatov reviewed Sep 7, 2023
View reviewed changes
Sources/PackageModel/IdentityResolver.swift Outdated
return nil
}

public static func sanitizeDependencyLocation(fileSystem: FileSystem, packageKind: PackageReference.Kind, dependencyLocation: String) throws -> String {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: missing doc comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think I am capable to write a doc comment for this, I do not understand the logic behind what it does.

@neonichu neonichu force-pushed the mirrors-for-root-dependencies branch from d31a027 to 2bdf203 Compare September 8, 2023 21:37
@neonichu
Copy link
Contributor Author

neonichu commented Sep 8, 2023

@swift-ci please test

@neonichu
Copy link
Contributor Author

neonichu commented Sep 8, 2023

@swift-ci test windows

@neonichu neonichu force-pushed the mirrors-for-root-dependencies branch from 2bdf203 to 5f1bd90 Compare September 8, 2023 21:47
@neonichu
Apply mirrors to root dependencies
bd0b44d 
We weren't applying mirrors to root dependencies so far.

rdar://110869499
@neonichu neonichu force-pushed the mirrors-for-root-dependencies branch from 5f1bd90 to bd0b44d Compare September 8, 2023 21:48
@neonichu
Copy link
Contributor Author

neonichu commented Sep 8, 2023

I forgot to add the new file...

@neonichu
Copy link
Contributor Author

neonichu commented Sep 8, 2023

@swift-ci please test

@neonichu
Copy link
Contributor Author

neonichu commented Sep 8, 2023

@swift-ci please test windows

@neonichu
Copy link
Contributor Author

neonichu commented Sep 8, 2023

Looks like there are relevant failures in the self-hosted job

@neonichu
Copy link
Contributor Author

I'm a bit confused about the self-hosted failures, I thought they were in integration tests, but seems like they're happening in the regular test suite?

@neonichu
Copy link
Contributor Author

@swift-ci please test macOS

@neonichu
Copy link
Contributor Author

Looks like they were flukes?

@neonichu neonichu merged commit 77e77a5 into main Sep 12, 2023
@neonichu neonichu deleted the mirrors-for-root-dependencies branch September 12, 2023 21:33
MaxDesiatov pushed a commit that referenced this pull request Sep 28, 2023
@neonichu @MaxDesiatov
Apply mirrors to root dependencies (#6765)
2e0c761 
We weren't applying mirrors to root dependencies so far.

rdar://110869499
MaxDesiatov pushed a commit that referenced this pull request Sep 28, 2023
@neonichu @MaxDesiatov
Apply mirrors to root dependencies (#6765)
fe5b366 
We weren't applying mirrors to root dependencies so far.

rdar://110869499
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MaxDesiatov MaxDesiatov MaxDesiatov left review comments

@tomerd tomerd tomerd approved these changes

@abertelrud abertelrud Awaiting requested review from abertelrud

Assignees

@neonichu neonichu

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@neonichu @MaxDesiatov @weissi @tomerd