swiftlang / swift-package-manager

The Package Manager for the Swift Programming Language
Apache License 2.0
9.75k stars 1.35k forks source link

Resolving tag with Semver Build Metadata is failing #6675

Open alexanderwe opened 1 year ago

alexanderwe commented 1 year ago

Description

I am not sure if I am doing something wrong, but it seems that SPM has some issues resolving package Semver compliant tags which have build metadata attached.

When I am using a tag, e.g 0.0.1-ms+20230704.1 it seems that this cannot be correctly resolved and then not deterministically download the correct version.

As an xample here:

Screenshot 2023-07-04 at 21 39 41

I want to resolve the tag 0.0.1-ms+20230704.1 of my test library which should be this commit https://github.com/alexanderwe/SPMTestLibrary/commit/3e80b7fe274193dcb325210c534d00e2d6027e5c, but SPM seems to assume it should be this commit https://github.com/alexanderwe/SPMTestLibrary/commit/f9c32ab925eb2bd71e92d07ede5bb33ee1595c6a which would be the tag 0.0.1-ms+20230705

I am not entirely sure if tags of this kind are supported by SPM.

Therefore my question here would be whether this should be supported and is a bug now, me doing something wrong or if this form of Semver tags is not supported by SPM.

Thanks a lot if you find time to take a look into this.

Expected behavior

I would expect that SPM can resolve the correct commit for the tag that's specified.

Actual behavior

The package resolution seems to not deterministically use the same commit when using a Semver version with build metadata attached.

Steps to reproduce

  1. Add the following test package to a sample project https://github.com/alexanderwe/SPMTestLibrary
  2. Use tag 0.0.1-ms+20230704.1
  3. Change the tag to 0.0.1-ms+20230705
  4. This should lead to resolution issues

Swift Package Manager version/commit hash

No response

Swift & OS version (output of swift --version ; uname -a)

swift-driver version: 1.75.2 Apple Swift version 5.8.1 (swiftlang-5.8.0.124.5 clang-1403.0.22.11.100) Target: arm64-apple-macosx13.0 Darwin ------- 22.5.0 Darwin Kernel Version 22.5.0: Thu Jun 8 22:22:20 PDT 2023; root:xnu-8796.121.3~7/RELEASE_ARM64_T6000 arm64

WowbaggersLiquidLunch commented 1 year ago

Therefore my question here would be whether this should be supported and is a bug now, me doing something wrong or if this form of Semver tags is not supported by SPM.

Yes this should be supported.

You might have tried it already, but just in case, does the problem go away if you delete the cache?

There are still some bugs in the SemVer parsing logic that I haven't found time to fix yet. Although, I don't think any of the bugs (that I know of) can result in the error you're seeing.

alexanderwe commented 1 year ago

I did try to reset package caches, clean derived data and also deleting the SPM caches. It then works for the first resolution of the package. But as soon as I change the tag the error starts popping up again.

Not sure, just a guess, could version comparison cause an issue ? I copied the Version.swift code from SPM into a Playground. It seems like Equatable protocol does not correctly compare the build meta data ? Might also be hard to actually decide which of the versions is "greater" or "lower" based on the build metadata

Screenshot 2023-07-05 at 09 06 34
WowbaggersLiquidLunch commented 1 year ago

It seems like Equatable protocol does not correctly compare the build meta data ?

What you see is the intended and correct behaviour. See the specification rule #10:

Build metadata MUST be ignored when determining version precedence. Thus two versions that differ only in the build metadata, have the same precedence. Examples: 1.0.0-alpha+001, 1.0.0+20130313144700, 1.0.0-beta+exp.sha.5114f85, 1.0.0+21AF26D3----117B344092BD.

WowbaggersLiquidLunch commented 1 year ago

I actually forgot about the build metadata comparison rule, until your experiment jogged my memory a bit. Now I see what's going on:

When you specify a version, SwiftPM looks for an equivalent from a repository's tags. Since build metadata does not factor into the equivalence comparison, when you have 2 (or more) tags with difference only in their build metadatas, SwiftPM can use any of them to satisfy the specified version requirement.

Maybe we can change the algorithm for choosing among equivalently-versioned tags, so it fits most people's expectations. Though, I don't think we should. It would be encouraging misusing SemVers. Also, it looks like (with half-guessing) SwiftPM currently uses the newest tag that satisfies the version requirement. This looks reasonable to me.

neonichu commented 1 year ago

It does seem as if exact should honor the build metadata as well. @tomerd wdyt?

tomerd commented 1 year ago

that seems reasonable but only when using exact

alexanderwe commented 1 year ago

@neonichu I would also argue that when one specifies one exact tag, exactly this tag should be resolved. At least this is what I would expect from an exact rule.

But for all the other cases I would agree that the Semver specification should be followed and when comparing tags build metadata is not considered and the latest tag of this equal tags is chosen, e.g. when using a upToNextMajor or when using a range. For these cases it would maybe help if this then could be documented (if it not already is)

WowbaggersLiquidLunch commented 1 year ago

There is already package(url:revision:) that can take an arbitrary tag, which can be used to sidestep the semantics of SemVer. I don't think we should give package(url:exact:) the same behaviour.

alexanderwe commented 1 year ago

Fair point. So basically this would mean package(url:revision:) bypasses SemVer semantics and should be used if I really only want this specific tag. And package(url:exact:) still says it will be exactly this tag, but following SemVer semantics which would mean, there could be a newer version which will then be used, correct ?

WowbaggersLiquidLunch commented 1 year ago

And package(url:exact:) still says it will be exactly this tag, but following SemVer semantics which would mean, there could be a newer version which will then be used, correct ?

package(url:exact:) takes a Version as the second argument, so it doesn't say anything about an exact tag. When you pass in a version string to this function, you're really passing in a Version instantiated through its ExpressibleByStringLiteral initialiser.

alexanderwe commented 1 year ago

Thanks a lot for the explanation. So to summarise, we should use:

package(url:revision:) - when really a specific commit is needed package(url:exact:) - when its okay that another commit is used, based on the SemVer specification (Which could lead to select a different commit, if only build metadata is different)

Would that be correct ?