potatoqualitee / psmodulecache

This action makes caching PowerShell modules from the PowerShell Gallery easy for Linux, Windows and macOS runners.
MIT License
31 stars 12 forks source link

Valid version said to be invalid #37

Closed potatoqualitee closed 1 year ago

potatoqualitee commented 2 years ago

@LaurentDardenne do you have time this week to check this out? Usually I'd be happy to but I'm trying to get a new dbatools library out. A valid version is being said to be invalid

The syntax of the version '2022.10.25.1' is invalid for 'dbatools-library:2022.10.25.1'.
The syntax of the version '2022.10.25.1' is invalid for 'dbatools-core-library:2022.10.25.1'

Also, I cannot get dbatools-library:: to work, even with single quotes around it. Any suggestions?

LaurentDardenne commented 2 years ago

A priori the string '2022.10.25.1' is not a valid semver :

2. A normal version number MUST take the form X.Y.Z where X, Y, and Z are non-negative integers, 
 and MUST NOT contain leading zeroes. X is the major version, Y is the minor version, and Z is the patch version.
Each element MUST increase numerically. For instance: 1.9.0 -> 1.10.0 -> 1.11.0.

With ''2022.10.25.1' the regex return $false .

I made the choice to use semver instead of [version], we can combine the two if we stay on 3 digits in the number. The [Version] class does not know how to manage the pre-release part, but 4 digit:

[version]'2022.10.25.1'
#ok

It's a format mixing puzzle and PSget seems to straddle both formats :-/ This test case does not exist.

Fixing this requires handling both formats in the version test.

It is necessary to manage the class [Version] OR the regex semver. Either it's semver with or without prerelease, or a version without prerelease I'm looking at the test data this weekend.

LaurentDardenne commented 2 years ago

We should have something like this, but I haven't tested it:

function Test-Version {
    #return $true for a valid version (without prerelease) or a valid semantic version (with or without prerelease).
    param(
        [string]$Version
    )
     #On ne sait pas encore quel format de version on manipule
    $isCLRVersion=$false

    #Est-ce une Semver ou une [Version] sur 3 digits ?
    $isSemverOrCLRVersion=$Version -match $script:SemverRegex

    if ($isSemverOrCLRVersion -eq $false)
    {
      #Ce n'est pas une Semver, mais est-ce une [Version] sur 4 digits?
     try { $Version=[Version]$Version } catch {$Version=$null}
     #$isCLRVersion est à $true si le cast réussi
     $isCLRVersion= $null -ne $Version
    }
     #Si ce n'est ni une Semver ni une version valide on renvoie $false
    return ($isSemverOrCLRVersion -OR $isCLRVersion)
}
LaurentDardenne commented 2 years ago

I adapted the test, as indicated above the versioning of the publication numbers can be done on both formats. There are therefore some collisions to manage, a valid number for one of the formats may be invalid for the other.

[String[]]$global:ValidSemanticVersions = @(
   #'1.2' invalid Semver but valid CLR version
   #'1' invalid for CLR version AND Semver
   '0.0.0' #valid CLR version
   '0.0.4', #valid CLR version
   '1.2.3', #valid CLR version
   '10.20.30', #valid CLR version
   '1.0.0', #valid CLR version
   '2.0.0', #valid CLR version
   '1.1.7', #valid CLR version
   ... #invalid CLR version
)

[String[]]$global:InvalidSemanticVersions = @(
   '0.0.0.0' #valid CLR version
   '0.0.-0' #valid CLR version !!
   '01.1.1', #valid CLR version
   '1.01.1', #valid CLR version
   '1.1.01', #valid CLR version
   '1.2', #valid CLR version
   '2022.1.2.3', #valid CLR version
   '1.0'  #valid CLR version
   '1.2', #valid CLR version
   ... #invalid semver
)

[String[]]$global:ValidClrVersions = @(
   #'1','11', '1.', '-1' are invalid for [version]'string'

   #'1.0' is valid but returns digits initialized to -1
   #  Major  Minor  Build  Revision
   #  -----  -----  -----  --------
   #  1      0      -1     -1
   #note : The value of Version properties that have not been explicitly assigned a value is undefined (-1).

   #[version]'1.0.-1' throw an exception
   #[version]'0.0.-0'  -> OK !!!
   '0.0.0.0'
   '1.2',
   '1.2.3',
   '2022.1.2.3' #It is a invalid Semver but a valid clr version.
)

There are also cases of the 'ValidSemanticVersionConstraints' test set to be excluded in the Test-Version and Test-PrereleaseVersion functions. We will not find these syntaxes in a nuget repository, but in the value of the parameter it remains valid. Finally for the syntax of a version number with prerelease, I don't know which ones are accepted by powershellget. That said, we only manage existing version numbers, therefore authorized on PSGallery. I will try to submit a PR this WE...

potatoqualitee commented 2 years ago

Thank you so much for the accommodation!

LaurentDardenne commented 2 years ago

The problem seems to be fixed: image

However I still have to perform a check on the 'ConvertTo-Version' function.

LaurentDardenne commented 2 years ago

I found this last night and it confirms my doubts about whether or not to follow PowershellGet version management. As I use a regex for semver v2, the minimal risk is to allow valid prerelease version numbers for PSModuleCache but which cannot exist in PSGallery. As a user, you enter an existing version number, except typing error.

I wonder if I'm not going to simplify this version management by reusing the powershell function mentioned above...

See too: https://learn.microsoft.com/en-us/nuget/concepts/package-versioning#where-nugetversion-diverges-from-semantic-versioning https://learn.microsoft.com/fr-fr/nuget/concepts/package-versioning#semantic-versioning-200

potatoqualitee commented 2 years ago

ohh, that is interesting! reusing their own command seems ideal

potatoqualitee commented 2 years ago

check it out, @LaurentDardenne ! We can now manage our caches

image

LaurentDardenne commented 2 years ago

Yes I saw but in an automation it does not help me, but we can already check the generation of the key name, which can help PSModuleCache users and others :-)

For this version problem which is to manage a specific [Version] + prerelease information I wanted to check if an external repository can contain modules with a different version format. That is to say delivered without using the Publish-Module cmdlet which controls the version format authorized on PSGallery. Then if we try to deliver a module with a 1.0 version, this cmdlet changes the version to 1.0.0, but there are modules on PSgallery with a 1.0 version delivered with a previous version of PSGet (I suppose). It is unlikely, but not impossible, to require such a version to be cached. It is also to be tested.

But being on a complicated professional project, I don't have the desire to code the Weekend at the moment. I would do that on the fly. If the management of version numbers '1.2.3.4' matters, we can however deliver what I had corrected, but just a patch in DIY mode ( pas sûr de la traduction :-) ).

LaurentDardenne commented 2 years ago

I just tested, so we can have any version on a module repository but only those respecting the PowershellGet versioning name can be managed :

image

Modules in version 1.0 can be saved. This seemed obvious, but we can now specify this behavior in the documentation. I will adapt the code using the function mentioned above.

LaurentDardenne commented 2 years ago

I plan to fix this problem in the next few days. To note : The following version numbers are valid but do not follow the standard defined for PowershellGet. Their use does not trigger a syntax error but returns 'NoMatchFoundForCriteria' :

'1.2-',
'1.2.3-',
'1.2.3--test',
'1.2.3.4-',
'1.2.3.4--',
'1.2.3- Beta1'

This syntax causes a bug when calling -split on a constrained variable:

[ValidateNotNullOrEmpty()][string]$Version='-1.2.3-'
$Version,$Prerelease = $Version -split '-',2
#Error: The variable cannot be validated because the value is not a valid value for the Version variable.

I leave the function code as is, it will remain consistent with PowershellGet.

potatoqualitee commented 1 year ago

this is fixed in the version i just merged?

LaurentDardenne commented 1 year ago

Yes it is fixed. As stated I haven't fixed the bugs in the code of the 'ValidateAndGet-VersionPrereleaseStrings' function. And I think most of the time it doesn't bother anyone.

LaurentDardenne commented 10 months ago

In addition. When searching for a dependent module, it seems that version '2.0.0' (key 'RequiredVersion') is not considered the same as '2.0'...

    RequiredModules = @( @{ ModuleName = 'modulerequirelicenseacceptance'; RequiredVersion = '2.0.0'; Guid = '8dba4e13-19fd-41b5-8840-246e2f2b30c7' } )
# Version number of this module.
ModuleVersion = '2.0'

GUID = '8dba4e13-19fd-41b5-8840-246e2f2b30c7'

# Author of this module
Author = 'farehar'

image

This case throw an exception : MissingMemberException: The required module 'modulerequirelicenseacceptance' with version '2.0.0' is not loaded

C'est bien dommage !