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

.pack installation failure #80

Closed
fred-r opened this issue May 11, 2022 · 8 comments · Fixed by #81
Closed

.pack installation failure #80

fred-r opened this issue May 11, 2022 · 8 comments · Fixed by #81
Assignees
Labels
bug Something isn't working

Comments

@fred-r
Copy link

fred-r commented May 11, 2022

> 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 jkrech added the bug Something isn't working label May 11, 2022
@jkrech
Copy link
Member

jkrech commented May 11, 2022

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
Copy link

Maybe linked to Open-CMSIS-Pack/devtools#329 ?

@chaws
Copy link
Contributor

chaws commented May 12, 2022

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

@jkrech
Copy link
Member

jkrech commented May 12, 2022

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
Copy link
Contributor

chaws commented May 12, 2022

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 Open-CMSIS-Pack/devtools#329 because from what I read briefly the piece of code in packchk, it does not use regex for parsing semantic version.

@jkrech
Copy link
Member

jkrech commented May 13, 2022

@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
Copy link
Contributor

chaws commented May 13, 2022

@jkrech Open-CMSIS-Pack/Open-CMSIS-Pack-Spec#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 chaws closed this as completed in #81 May 13, 2022
@chaws
Copy link
Contributor

chaws commented May 13, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants