Skip to content

Changes in Clang do not rebuild Clad #7977

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

Closed
hahnjo opened this issue Apr 23, 2021 · 6 comments
Closed

Changes in Clang do not rebuild Clad #7977

hahnjo opened this issue Apr 23, 2021 · 6 comments

Comments

@hahnjo
Copy link
Member

hahnjo commented Apr 23, 2021

Describe the bug

As seen in #7488, changing headers / data structures in Clang doesn't rebuild Clad and causes very weird test failures.

Expected behavior

The build system should detect the changes and rebuild Clad as well as libCling.so as needed.

To Reproduce

Apply / Revert #7488 to see a crash or just touch Clang header files and observe that Clad isn't rebuilt.

@hahnjo
Copy link
Member Author

hahnjo commented Apr 23, 2021

Not sure who to assign, feel free to change...

@oshadura oshadura assigned bellenot and unassigned oshadura Jul 14, 2021
@vgvassilev
Copy link
Member

I am not sure how we can fix this issue. The cmake ExternalProject_Add is meant to operate on projects which are not changed in tree. There seems to be an option: BUILD_ALWAYS which says " Enabling this option forces the build step to always be run. This can be the easiest way to robustly ensure that the external project's own build dependencies are evaluated rather than relying on the default success timestamp-based method. This option is not normally needed unless developers are expected to modify something the external project's build depends on in a way that is not detectable via the step target dependencies (e.g. SOURCE_DIR is used without a download method and developers might modify the sources in SOURCE_DIR)."

That would fix this problem but introduce another one -- clad would be rebuilt every time one types make. ROOT has good amount of uses of ExternalProject_Add and that kind of issue is not only specific to clad.

I think we have several options: a) close the bug and ignore that type of errors (which can waste dev debug time); b) add BUILD_ALWAYS and waste a lot of user/dev time; c) check if it is feasible to somehow track changes in the source code within the regular cmake (which probably has proven hard and people added the BUILD_ALWAYS option).

@hahnjo
Copy link
Member Author

hahnjo commented Oct 5, 2021

I'm ok to close this; it will probably pop up again in the future, but I agree that rebuilding Clad every time is a higher time sink for the team than one developer having to investigate weird failures. And by now, we hopefully have a sufficient understanding that it will take less time in the future because we know how to "fix" it.

@dpiparo
Copy link
Member

dpiparo commented Feb 3, 2024

Since @hahnjo is OK to close it, we close it. Please do not hesitate to re-open in case more information becomes available that suggests to revisit this decision.

@vgvassilev
Copy link
Member

Let's close it. It is annoying but the ExternalProject_Add is supposed to bring in projects as they are and it does not expect to have development or any major changes in there.

@hahnjo
Copy link
Member Author

hahnjo commented Feb 5, 2024

Ok, but it's not fixed in 6.30.06! I've adjusted the projects accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

5 participants