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

GCC_PKGREL must follow semver rules #14

Merged
merged 1 commit into from
Aug 4, 2020

Conversation

mcspr
Copy link

@mcspr mcspr commented Aug 4, 2020

Replace leading zeroes with minor.major.patch 0-padding, because PlatformIO uses semantic version module for package version sorting.
Meaning, future 10.2 must be 100200

see platformio/platformio-core#3612 (comment)

Yeah, replacing the exact version match with the >=5 (as it is here https://github.com/Jason2866/platform-espressif8266/blob/b0167d26f3708396942ce6b663544e4984490ca5/platform.json#L39), I do see an error:

File "/home/builder/git/platformio-core/platformio/managers/package.py", line 118, in max_satisfying_repo_version
  specver = semantic_version.Version(v["version"])
File "/home/builder/.platformio/penv/lib64/python3.8/site-packages/semantic_version/base.py", line 105, in __init__
  major, minor, patch, prerelease, build = self.parse(version_string, partial)
File "/home/builder/.platformio/penv/lib64/python3.8/site-packages/semantic_version/base.py", line 318, in parse
  raise ValueError("Invalid leading zero in minor: %r" % version_string)
ValueError: Invalid leading zero in minor: '5.01010.200706'

But it is a typo... from the esp-quick-toolchain side? Meaning 01010 should be 100100, same as the version uploaded into the PIO registry and conforming with semver:
https://semver.org/#spec-item-2 (quoting ...MUST NOT contain leading zeroes...)
https://bintray.com/platformio/dl-packages/toolchain-xtensa/2.100100.200706

Effectively, this does nothing atm, because we don't have a list of available versions and install happens by directly setting the version. I'd wait until response from PIO guys though, whether this package deployment can be simplified
(perhaps we will get a 'public' registry a-la npm or the option I proposed in the pio-core issue tracker)

Also, so I don't forget this fix is needed.

Replace leading zeroes with minor.major.patch 0-padding
@@ -30,7 +30,7 @@ GNUHTTP := https://gcc.gnu.org/pub/gcc/infrastructure
ifeq ($(GCC),4.8)
ISL := 0.12.2
GCC_BRANCH := call0-4.8.2
GCC_PKGREL := 00482
GCC_PKGREL := 40802
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@earlephilhower
Copy link
Owner

On a related note, does that PKGREL value need to update on updates? How will the PIO stuff handle, say, fixing a bug in newlib and releasing another 100100 version?

@mcspr
Copy link
Author

mcspr commented Aug 4, 2020 via email

Copy link
Owner

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Will need a full toolchain build to be integrated to the packages.

@earlephilhower earlephilhower merged commit 1917a87 into earlephilhower:master Aug 4, 2020
@mcspr mcspr deleted the pio/semver branch August 28, 2020 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants