Open-CMSIS-Pack / cpackget

Open-CMSIS-Pack Package Installer
Apache License 2.0
20 stars 13 forks source link

.pack installation failure #80

Closed fred-r closed 2 years ago

fred-r commented 2 years ago
> cpackget.exe pack add STMicroelectronics.STM32U5xx_Drivers.2.0.0-alpha.0.5.pack
I: Using pack root: "C:\CMSIS_PACKS"
I: Adding pack "STMicroelectronics.STM32U5xx_Drivers.2.0.0-alpha.0.5.pack"
E: pack version not found in the pdsc file

$ echo $CMSIS_PACK_ROOT
C:\CMSIS_PACKS

$ ls /c/CMSIS_PACKS/.Local/STM*
/c/CMSIS_PACKS/.Local/STMicroelectronics.Example_HAL_GPIO_Toggle.pdsc
/c/CMSIS_PACKS/.Local/STMicroelectronics.board_resources_B-U585I-IOT02A.pdsc

$ ls /c/CMSIS_PACKS/.Web/STM*
/c/CMSIS_PACKS/.Web/STMicroelectronics.STM32U5xx_Drivers.pdsc

frq09468@LMECWL0888 MINGW64 /c/Packs
$ cat /c/CMSIS_PACKS/.Web/STMicroelectronics.STM32U5xx_Drivers.pdsc
<?xml version="1.0" encoding="UTF-8"?>

<package schemaVersion="1.6.0" xmlns:xs="http://www.w3.org/2001/XMLSchema-instance" xs:noNamespaceSchemaLocation="PACK.xsd" Dname="STM32U5*">
  <vendor>STMicroelectronics</vendor>
  <name>STM32U5xx_Drivers</name>
  <description>STMicroelectronics STM32U5 series HAL and LL</description>
  <url>xxxxxxxxxx</url>

  <releases>
    <release version="2.0.0-alpha.0.5" date="2022-04-28">Fix few issues</release>

And:

> cpackget.exe --version
cpackget version 0.4.1
jkrech commented 2 years ago

As far as I can see, this seems to be a bug. Hopefully this is not due to the fact that the version 2.0.0-alpha.0.5 uses dots after the dash, which is compliant to the Semantic Version format.

jeromecoutant commented 2 years ago

Maybe linked to https://github.com/Open-CMSIS-Pack/devtools/issues/329 ?

chaws commented 2 years ago

This does seem like a bug parsing the file name, I'm still investigating

jkrech commented 2 years ago

I see. I guess we must not read the version backwards from the last "." prior to the file extension (.pack .zip). Instead we rely on the "." as separator between <vendor>.<name>.<version>.[pack|zip] This means <vendor> and <name> must not contain any "." so version starts after second "." and we strip .[pack|zip] to get to extract that complete semantic version.

chaws commented 2 years ago

OK, I think I have found the reason. Cpackget uses the regex for semantic versioning from packIndexFile spec and I have figured that it is matching more than it should:

From the regex in PackIndexFile spec, this is the pattern snippet for the "quality" part of the version (note that there is 3 capturing groups, but the last matches many due to "*", causing it to generate a variable number of matches):

 -                      alpha                   .0 (.5 is a separate match due to *)
(-(0|[1-9][0-9]*|[0-9]*[a-zA-Z-][0-9a-zA-Z-]*)(\.(0|[1-9][0-9]*|[0-9]*[a-zA-Z-][0-9a-zA-Z-]*))*)?

Running it against "-alpha.0.5" will generate 4 matches: "-", "alpha", ".0" and ".5" (https://regex101.com/r/BO76pa).

Now take a look at the same regex snippet from Semver.org:

   -                   alpha                     .0 (.5 matches due to *, except that now it belongs to the broader match "alpha.0.5")
(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?

The extra ?: transforms the inner regex into a non-capturing group telling the engine not to include that in the matches, leaving the entire snippet with only a single fixed capturing group. If we run the same regex on "-alpha.0.5", we will get a single match "alpha.0.5" (https://regex101.com/r/f4QiL3/1).

Cpackget uses this regex to both match a given pack version and also to separate it from the pack name. The latter was buggy because of the uncertain number of matches that the regex could generate. After swapping the regex to Semver.org one, the bug was fixed! I'll be sending a PR soon.

@jeromecoutant I don't think this is related to https://github.com/Open-CMSIS-Pack/devtools/issues/329 because from what I read briefly the piece of code in packchk, it does not use regex for parsing semantic version.

jkrech commented 2 years ago

@chaws thanks a lot for the investigation. Could you please also raise a PR against the PACK.xsd to get the expression there to match the one from semver.org. Please include a link in the comment which reference you have been using. Thanks a lot.

chaws commented 2 years ago

@jkrech https://github.com/Open-CMSIS-Pack/Open-CMSIS-Pack-Spec/pull/123, I could not find a way to test if things break with the change though.

Oops, scratch that. I saw CI at work :) I'll fix the PR

chaws commented 2 years ago

I have merged the bugfix for this issue. I will rollout a new release of cpackget next week.