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

Don't set `CPACK_RPM_MAIN_COMPONENT' for ASAN builds #212

Merged
merged 1 commit into from
Jul 23, 2024

Conversation

lawruble13
Copy link
Collaborator

Setting CPACK_RPM_MAIN_COMPONENT to any component causes the package name for that component to be automatically generated from CPACK_PACKAGE_NAME (ignoring the other work we've done to correct the name), which in particular causes the runtime packages for ASAN builds to not be generated with the ASAN suffix.

In order to minimize the impact, I'm disabling this call only in the case that we're doing an ASAN build, I don't want to break any other spots where it is actually working correctly, but I think we should investigate removing this entirely.

Setting `CPACK_RPM_MAIN_COMPONENT` to any component causes the package name for that component to be automatically generated from `CPACK_PACKAGE_NAME` (ignoring the other work we've done to correct the name), which in particular causes the runtime packages for ASAN builds to not be generated with the ASAN suffix.

In order to minimize the impact, I'm disabling this call only in the case that we're doing an ASAN build, I don't want to break any other spots where it is actually working correctly, but I think we should investigate removing this entirely.
Copy link
Collaborator

@cgmb cgmb left a comment

Choose a reason for hiding this comment

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

Is the static build going to have the same issue, as it also has a suffix?

It appears that the docs are failing to build on Ubuntu 20.04, but this appears to be a pre-existing issue, as it occurs on the branch as well.

Note that the CMake behaviour was introduced in CMake 3.8, and we use a newer version of cmake than that on all platforms so we can expect this behaviour to be consistent.

Copy link
Collaborator

@cgmb cgmb left a comment

Choose a reason for hiding this comment

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

Is the static build going to have the same issue, as it also has a suffix?

Oh. Of course not, as there is no runtime component. LGTM.

@lawruble13
Copy link
Collaborator Author

Oh. Of course not, as there is no runtime component. LGTM.

Was typing pretty much exactly this, beat me to it 😄

I was less concerned with inconsistent behaviour across CMake versions, and more across different builds. I think everything we've got set up should ensure the runtime package ends up always being named without a suffix, but I couldn't test that to verify at the time and we're short on time to have our fix cause something else to break.

@pramenku
Copy link

Also @lawruble13 Please get this PR merge to Mainline using regular promotion process ASAP. Thanks,

@mamaydeo mamaydeo merged commit 1862031 into release/rocm-rel-6.2 Jul 23, 2024
5 of 15 checks passed
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.

4 participants