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

[5.10] Fix non-tarballed SDK installation with remote URL (#7312) #7321

Conversation

kateinoigakukun
Copy link
Member

@kateinoigakukun kateinoigakukun commented Feb 5, 2024

🍒 #7312

  • Explanation: 4714ea9 introduced a regression where non-tarball SDKs could not be installed from a remote URL due to the wrong assumption that the downloaded file would always be a tarball.
  • Scope: Experimental Swift SDK installation
  • Risk: Low, only affects to swift experimental-sdk command, which is still experimental. Also doesn't impact other binaries or SwiftPM clients/libraries.
  • Testing: Some new unit tests are added in this change.
  • Reviewer: @MaxDesiatov
  • Main branch PR: Fix non-tarballed SDK installation with remote URL #7312

### Motivation:

4714ea9 introduced a regression where non-tarball SDKs could not be
installed from a remote URL due to the wrong assumption that the
downloaded file would always be a tarball.

This issue exists in 5.10 release branch, so will cherry-pick after
merging this PR.

### Modifications:

This commit fixes the issue by checking the file extension part of the
URL for every supported archive format, then falling back to the tarball
format if no supported extension is found.

### Result:

Non-tarball SDK artifact bundles can be installed from remote URL.
@kateinoigakukun
Copy link
Member Author

@swift-ci Please test

@MaxDesiatov
Copy link
Member

@kateinoigakukun would you mind adding a filled in form to the PR description with Explanation/Scope/Risk/Testing etc fields matching recent 5.10 PRs, like this one for example? #7308

@kateinoigakukun
Copy link
Member Author

Gentle ping :)

@MaxDesiatov
Copy link
Member

Unfortunately, it doesn't look this will make it in time for 5.10.0 as the bar for that is very high. It may still be eligible for subsequent patch 5.10.x releases if those happen, so I'll keep this PR open for now.

@kateinoigakukun
Copy link
Member Author

@swift-ci test

MaxDesiatov
MaxDesiatov previously approved these changes Mar 19, 2024
MaxDesiatov

This comment was marked as duplicate.

@MaxDesiatov MaxDesiatov dismissed their stale review March 19, 2024 08:31

We need to hold off merging this unfortunately for a bit more time. There are infrastructure changes requires for this to land correctly post 5.10.0

@MaxDesiatov MaxDesiatov merged commit a0e7a8a into apple:release/5.10 Apr 3, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cross-compilation swift 5.10 This PR targets the 5.10 branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants