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

SwiftPM binary target checksum catch-22 #1674

Closed
WowbaggersLiquidLunch opened this issue Nov 14, 2020 · 25 comments · Fixed by #1710
Closed

SwiftPM binary target checksum catch-22 #1674

WowbaggersLiquidLunch opened this issue Nov 14, 2020 · 25 comments · Fixed by #1710
Labels
1.x Sparkle 1.x

Comments

@WowbaggersLiquidLunch
Copy link

WowbaggersLiquidLunch commented Nov 14, 2020

I am trying to move the Sparkle dependency in the IINA project from CocoaPods- to SwiftPM-managed (iina/iina#2877). Every time when I ask Xcode to add Sparkle, it says it can not download the artifact (which I assume is the Zip archive?). Here is the output from Xcode, edited to include only Sparkle-related entries:

Fetching https://github.com/sparkle-project/Sparkle
Cloning https://github.com/sparkle-project/Sparkle
Checking out https://github.com/sparkle-project/Sparkle at 1.24.0
Removing https://github.com/sparkle-project/Sparkle
artifact of binary target 'Sparkle' failed download: invalid status code 404

The 404 code seems like Xcode can not find the artifact.

I tried on both Xcode 12.2 and 12.3 Beta (12C5020f).

@WowbaggersLiquidLunch WowbaggersLiquidLunch added the 1.x Sparkle 1.x label Nov 14, 2020
@WowbaggersLiquidLunch
Copy link
Author

cc @thebluepotato

@thebluepotato
Copy link
Contributor

Yes it seems the SPM zip archive was not added to the assets on the release page (only the standard tar.xz). @kornelski could you please add the Sparkle-SPM-1.24.0.zip that was produced when you ran the release script?

@kornelski
Copy link
Member

Try now.

@kornelski
Copy link
Member

kornelski commented Nov 15, 2020

Oh, I had to rebuild, which changed the checksum.

Try using the latest commit 891afd4

If you add a package to be from the master branch, instead of a tagged version, it will work now.

@WowbaggersLiquidLunch
Copy link
Author

It works now. Thanks!

Could 891afd4 be tagged as version 1.24.1, so that it could be automatically version-controlled by SwiftPM?

@thebluepotato
Copy link
Contributor

@WowbaggersLiquidLunch I guess it would be possible but just as a small warning to the repo owners to update the Xcode project version accordingly and always make sure to upload the SPM zip file alongside the usual assets. I actually didn't know if the release on Github is automated or not, but if yes, it would make sense to include the SPM assets in it.

@kornelski
Copy link
Member

Unfortunately, there's a bit of a chicken-egg problem with SPM. It wants Package.swift file with the checksum of the released version in the repo.

This means I have to first build a release, update Package.swift, and then commit and tag. But the current build system checks if the tag exists before building.

@thebluepotato
Copy link
Contributor

Would moving the tag one commit further after the make-release-package.sh script solve this? In other words, commit, tag version, run the set-git-version script, make release, commit again and force re-tag?

@WowbaggersLiquidLunch
Copy link
Author

WowbaggersLiquidLunch commented Nov 15, 2020

I searched a bit on the internet, and found these 2 articles:

https://git-scm.com/book/en/v2/Git-Internals-Git-Objects

https://gist.github.com/masak/2415865

I only took a cursory glance at them, but they seem to suggest that it is possible to calculate the checksum before committing. Still, it would still be too much of a hack. Maybe we should ask on the Swift forums or file a bug report?

EDIT: This won't work, because the checksum needs to be part of the diff.

@michelf
Copy link
Contributor

michelf commented Nov 15, 2020

I think the general idea is that a binary package should be a separate repository from the source. There's no source package at the moment, but if there was one it probably should be this repository. A binary package could be provided by a separate repository; that'd solve the chicken and egg problem.

@tonyarnold
Copy link
Contributor

I'm seeing the same thing, but with the 2.x branch. The file referenced from within the Swift package manifest doesn't exist on the server (404): https://github.com/sparkle-project/Sparkle/releases/download/2.0.0/Sparkle-SPM-2.0.0.zip

@thebluepotato
Copy link
Contributor

I think the general idea is that a binary package should be a separate repository from the source. There's no source package at the moment, but if there was one it probably should be this repository. A binary package could be provided by a separate repository; that'd solve the chicken and egg problem.

Indeed the main purpose of binary packages is for distribution of closed-source binaries but it can be used for other purposes such as here (bundling multiple apps, scripts and binaries). I think a separate repo would be a valid last-resort solution.

@thebluepotato
Copy link
Contributor

@kornelski What are the exact steps taken for a release of Sparkle? I was mostly aware of the Makefile only so I didn't think to check the git version script. When is it called?

To avoid the chicken-egg problem, we should either rethink the versioning process (for example having the xcodeproj version as the source of truth), maybe tag after the release process or as I suggested above, keep everything as-is and merely commit Package.swift and force a re-tag before pushing. I think that last one would be an easy workaround. What do you think?

@kornelski
Copy link
Member

The make release uses git describe to embed Sparkle version.

@ryanfrancesconi
Copy link

i see the same, adding via SPM and specified branch (master) works, adding from version has a 404

@kornelski
Copy link
Member

Yes, that's expected. Adding from version is going to fail until the next release. Please add the branch or the latest commit.

@gregcotten
Copy link

Hmm... Should probably version up with this fix, if possible. Most people won't want to use the SPM version pointing at a branch or commit.

@WowbaggersLiquidLunch WowbaggersLiquidLunch changed the title Xcode fails to download artifact of binary target SwiftPM checksum catch-22 Nov 25, 2020
@WowbaggersLiquidLunch WowbaggersLiquidLunch changed the title SwiftPM checksum catch-22 SwiftPM binary target checksum catch-22 Dec 5, 2020
@gregcotten
Copy link

Currently just pointing at commit 891afd4, but should I be pointing at master instead? How dangerous might that be?

@kornelski
Copy link
Member

kornelski commented Dec 5, 2020

What danger do you have in mind?

I don't force-push commits that have been tagged, so the commit won't disappear. In git, content of tags can be changed, but content of commits referenced by hash can't, so using commit hash is the strongest version-locking guarantee git can offer.

@gregcotten
Copy link

What danger do you have in mind?

I don't force-push commits that have been tagged, so the commit won't disappear. In git, content of tags can be changed, but content of commits referenced by hash can't, so using commit hash is the strongest version-locking guarantee git can offer.

Oh, all I am asking is if pointing at master instead of a specific commit would be a bad idea.

@orchetect
Copy link

orchetect commented Jan 3, 2021

SPM in Xcode works great when pointing at the master branch (version 1.x, no status code 404)

However, if I decide to add the SPM dependency by using the 2.x branch, it fails with status code 404 again because the zip file the Package.swift points to does not exist:

https://github.com/sparkle-project/Sparkle/releases/download/2.0.0/Sparkle-SPM-2.0.0.zip

Looks like 2.x is not quite ready for SPM use.

@kornelski
Copy link
Member

kornelski commented Jan 4, 2021

Yes, this is expected. There are no 2.x releases yet. SPM for 2.x will not work until there's an official release on GitHub, and this won't happen until #1523 is resolved.

thebluepotato added a commit to thebluepotato/Sparkle that referenced this issue Jan 4, 2021
@thebluepotato
Copy link
Contributor

thebluepotato commented Jan 4, 2021

@kornelski I've begun working on a fix but I'm still not sure about the order in which things are currently done. From what I can gather from set-git-version-info.sh and from some testing, here are the current steps for a release:

  1. Commit latest changes
  2. Bump SPARKLE_VERSION_(MAJOR/MINOR/PATCH) in ConfigCommon.xcconfig and s.version in Sparkle.podspec (e.g., 1.24.1)
  3. Commit
  4. Tag with new version number
  5. Run make release which runs the Distribution target (calling set-git-version-info.sh in certain sub-targets) and running make-release-package.sh
  6. Push everything to remote and upload assets

If that's correct, I would like to add something like git tag -fa <new_version> (original tag message is kept anyway) between steps 5 and 6 (i.e., before pushing). Since neither the tag nor the commits have been pushed to the remote at that stage, nothing bad can happen. The commit history is kept intact, only the tag moves. Another way to look at it is that we "prematurely tag" for the purposes of set-git-version-info.sh but the real tagging is after make-release-package.sh.

If you can confirm these steps, I can have a PR ready quite quickly and test it out.

@kornelski
Copy link
Member

Yeah, moving a tag can work.

@WowbaggersLiquidLunch
Copy link
Author

@kornelski can we get a new (patch?) release now that #1710 is merged?

lluuaapp pushed a commit to elgatosf/Sparkle that referenced this issue Aug 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x Sparkle 1.x
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants