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

Allow to specify a different build number for a single package and remove full_rebuild and skip_all_deps options #67

Merged
merged 3 commits into from
Jan 15, 2025

Conversation

traversaro
Copy link
Member

This commit allow to specify a different build number for a single package, to allow to perform a fix to a build of a single package, without the need to do a full rebuild.

This is an advanced option, so special care is needed:

  • A rebuild is only possible if it does not change the version or in any other way the ABI of the package.
  • On the following full rebuild, it is important to make sure that the build number select is higher then any custom build number used for single packages.

To permit to do this, I did the following changes:

  • I added an optional pkg_additional_info.yaml file meant to specify additional package-specific options. I introduced this file here, but it will also be useful for the work I plan to do to easily de-vendor packages (together with ament_vendor: Add AMENT_VENDOR_NEVER_VENDOR ament/ament_cmake#552)
  • There was a whole strange logic related to the selected_bn variable that I did not fully get, that was using a different build number depending on the build number of the packages present in the target upload channel. I was a bit afraid to simply remove it, but I added the use_explicit_build_number: True option to bypass this logic, the idea is that we add use_explicit_build_number: True to all the vinca config files, and then if everything works fine, we can simply drop that logic.

The motivation for this came out as in RoboStack/ros-humble#242 we accidentally did a build of lanelet2-io with a wrong pugixml pinning, and in general it would be great to be able to do fixes that preserve the ABI without doing a full rebuild.

vinca/main.py Outdated Show resolved Hide resolved
This commit allow to specify a different build number for a single
package, to allow to perform a fix to a build of a single package,
without the need to do a full rebuild.

This is an advanced option, so special care is needed:
* A rebuild is only possible if it does not change the version
  or in any other way the ABI of the package.
* On the following full rebuild, it is important to make sure that
  the build number select is higher then any custom build number used for single packages.
@traversaro traversaro force-pushed the allowcustombuildnumberforsinglepackage branch from eaa5524 to cc79494 Compare January 14, 2025 21:12
@Tobias-Fischer
Copy link
Contributor

I really like this change – it simplifies things a lot! Should we use this opportunity to do some more cleanup?

  • I think we should just make the use_explicit_build_number: True the default now as opposed to increasing the complexity with the old logic that has always been a little odd as it has evolved over time.
  • We should also assume that full_rebuild: True and remove any logic for the False case. Setting it to False will not be necessary anymore (see below)
  • Finally, let's also assume that skip_all_deps: false - this was previously used in combination with full_rebuild: false to rebuild just a few packages, but your proposed solution is much cleaner.

@traversaro
Copy link
Member Author

I really like this change – it simplifies things a lot! Should we use this opportunity to do some more cleanup?

* I think we should just make the `use_explicit_build_number: True` the default now as opposed to increasing the complexity with the old logic that has always been a little odd as it has evolved over time.

* We should also assume that `full_rebuild: True` and remove any logic for the `False` case. Setting it to `False` will not be necessary anymore (see below)

* Finally, let's also assume that `skip_all_deps: false` - this was previously used in combination with `full_rebuild: false` to rebuild just a few packages, but your proposed solution is much cleaner.

I done the first point, I look on the other points. For the other two points, I should completly remove the full_rebuild and skip_all_deps options?

@traversaro
Copy link
Member Author

For skip_all_deps, it is currently used in the migrate tool (see

vinca_conf["skip_all_deps"] = True
), not sure about that.

@Tobias-Fischer
Copy link
Contributor

Yes, I think we should completely remove full_rebuild and skip_all_deps for simplicity sake. We've never used the migrate tool, and if we need it, it should be easy to change it to instead write into the pkg_additional_info.yaml file in the future. This would be in line with your comments in #64 anyhow I think :)

@traversaro
Copy link
Member Author

Yes, I think we should completely remove full_rebuild and skip_all_deps for simplicity sake. We've never used the migrate tool, and if we need it, it should be easy to change it to instead write into the pkg_additional_info.yaml file in the future. This would be in line with your comments in #64 anyhow I think :)

Good point! The migrate tool should bump the build number of the affected packages.

@Tobias-Fischer
Copy link
Contributor

I'm happy to merge as soon as you are; I've done a quick update to #64 to write into pkg_additional_info.yml.

@traversaro
Copy link
Member Author

Yes, I think we should completely remove full_rebuild and skip_all_deps for simplicity sake. We've never used the migrate tool, and if we need it, it should be easy to change it to instead write into the pkg_additional_info.yaml file in the future. This would be in line with your comments in #64 anyhow I think :)

Done in debb33e . The PR is ready for merge.

@traversaro traversaro changed the title Allow to specify a different build number for a single package Allow to specify a different build number for a single package and remove full_rebuild and skip_all_deps options Jan 15, 2025
@Tobias-Fischer Tobias-Fischer merged commit f3e13bb into master Jan 15, 2025
@Tobias-Fischer Tobias-Fischer deleted the allowcustombuildnumberforsinglepackage branch January 15, 2025 19:05
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.

2 participants