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

release-repo-scripts Edgecase Collection #689

Open
3 of 5 tasks
methylDragon opened this issue Apr 7, 2022 · 3 comments
Open
3 of 5 tasks

release-repo-scripts Edgecase Collection #689

methylDragon opened this issue Apr 7, 2022 · 3 comments
Assignees

Comments

@methylDragon
Copy link
Contributor

methylDragon commented Apr 7, 2022

Description

This issue tracks all the various edgecases I've encountered in the ign to gz migration, running the scripts in https://github.com/ignition-tooling/release-tools/tree/master/release-repo-scripts

Scripts

bump_dependency.bash

  • Cases like ign-tools2 2.0.0 -> ign-tools3 2.0.0 don't bump properly. (2.0.0 is expected to be bumped to 3.0.0)
  • (NOT HANDLED! SEE CAVEAT/NOTES.): Case: IGN_<LIB>_VER N-1 -> IGN_<LIB>_VER N is not addressed
    • Fix some bump_dependency edgecases #699
    • It's a bad pattern, better handled with:
      IGN_<LIB>_VER N-1 -> IGN_<LIB>_VER ${ignition-<lib><N>_VERSION_MAJOR}
      • However, that means that the libraries have to be found BEFORE the variable is set. Using the script to fix this won't work because the script won't know how to reorder the lines.
      • Fix: Manually update the examples or any other instances to eliminate the pattern
  • Greedy matching matches non-library references
    • See example
    • Maybe check for string prepended with -?

new_ignition_release_repos.bash

  • If a library reference doesn't have a number suffix (ign-tools vs ign-tools1), the script can't pick it up
    • L78 is responsible
    • Most egregiously occurs with instances of "plugin"
    • There are many cases where we want to keep a library reference without a version! So it's not as simple as appending a version number...
    • Interim solution: Run grep -r "ign-<lib>[^0-9]" and inspect for anything missed
@scpeters
Copy link
Contributor

Case: IGN_VER N-1 -> IGN_VER N is not addressed

where is an example of this? I often see this as set(IGN_PLUGIN_VER ${ignition-plugin1_VERSION_MAJOR}), which I think is better, since it should be more compatible with the autocompletes

@methylDragon
Copy link
Contributor Author

methylDragon commented Apr 12, 2022

Typically appears in examples: https://github.com/ignitionrobotics/ign-rendering/search?q=IGN_PLUGIN_VER

I suppose it could be IGN_<LIB>_VER <any_digit> -> IGN_<LIB>_VER ${ignition-<lib>N_VERSION_MAJOR} instead? Though it'll remove affect any deliberate pins.

PR implementing this: #700 (with a bump)

@scpeters
Copy link
Contributor

Typically appears in examples: https://github.com/ignitionrobotics/ign-rendering/search?q=IGN_PLUGIN_VER

I suppose it could be IGN_<LIB>_VER <any_digit> -> IGN_<LIB>_VER ${ignition-<lib>N_VERSION_MAJOR} instead? Though it'll remove affect any deliberate pins.

PR implementing this: #700 (with a bump)

here's what I mean: gazebosim/gz-rendering#607

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

No branches or pull requests

2 participants