# Build when git commit is changed

**URL:** <https://discourse.slicer.org/t/build-when-git-commit-is-changed/29607>\
**Category:** Development\
**Created:** [May 23, 2023, 2:12pm UTC](https://discourse.slicer.org/t/build-when-git-commit-is-changed/29607 "2023-05-23T14:12:46Z")\
**Posts on this page:** 20\
**Page:** 1

<div class="post-metadata">

**Author:** ![aymeric.chataigner](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/aymeric.chataigner/32/4977_2.png) [@aymeric.chataigner](https://discourse.slicer.org/u/aymeric.chataigner)\
**Post date:** [May 23, 2023, 2:12pm UTC](https://discourse.slicer.org/t/build-when-git-commit-is-changed/29607/1 "2023-05-23T14:12:46Z")

</div>

Hi,

3D slicer incremental build takes 8 minutes on my Linux PC if I simply put my git branch on another commit (even if this commit does not change any c++ code).

- Config: i7 12 cores and 32GB RAM
- Build launch with this command in Slicer-build directory: make -j12

It is caused by Slicer-build/vtkSlicerVersionConfigure.h which is modified each time the commit is changed in the source directory.

We could store the constants of vtkSlicerVersionConfigure.h in a file read once when Slicer is launched to save build time without any performance drawback.

What do you think ?

Any advice to speed up build is welcome 🙂

Best regards

---

<div class="post-metadata">

**Author:** ![lassoan](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/lassoan/32/13_2.png) [@lassoan](https://discourse.slicer.org/u/lassoan)\
**Post date:** [May 23, 2023, 6:06pm UTC](https://discourse.slicer.org/t/build-when-git-commit-is-changed/29607/2 "2023-05-23T18:06:26Z")

</div>

vtkSlicerVersionConfigure.h is needed for some #ifdefs that are used at compile time, so it would not be sufficient to make it available in a resource file only.

That said, a change in `vtkSlicerVersionConfigure.h` should not cause much rebuilding, as it is just used in a few cxx files (also in `qSlicerAbstractModule.h`, which could be probably removed), so it will just build and link a handful of libraries.

Could you please check if build takes a long time if you just update `vtkSlicerVersionConfigure.h`?  
If that does not take too long then it may be something else triggers the lenghty recompilations/linkings.

---

<div class="post-metadata">

**Author:** ![pieper](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/pieper/32/8_2.png) [@pieper](https://discourse.slicer.org/u/pieper)\
**Post date:** [May 23, 2023, 6:20pm UTC](https://discourse.slicer.org/t/build-when-git-commit-is-changed/29607/3 "2023-05-23T18:20:37Z")

</div>

Any fixes that speed up incremental rebuilds would be very welcome.

---

<div class="post-metadata">

**Author:** ![aymeric.chataigner](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/aymeric.chataigner/32/4977_2.png) [@aymeric.chataigner](https://discourse.slicer.org/u/aymeric.chataigner)\
**Post date:** [May 24, 2023, 9:38am UTC](https://discourse.slicer.org/t/build-when-git-commit-is-changed/29607/4 "2023-05-24T09:38:18Z")

</div>

> [@lassoan](#):
>
> vtkSlicerVersionConfigure.h

Thanks for your quick answer.  
Incremental build still takes the same time if I just update vtkSlicerVersionConfigure.h.  
The first file recompiled is vtkMRMLVolumeSequenceStorageNode.cxx which is compiled quite quickly but then many linking must be done since it is in libMRMLCore which is used in many libs.  
In the next days I will try to minimize the use of #include “vtkSlicerVersionConfigure.h” and propose a pull request.

---

<div class="post-metadata">

**Author:** ![lassoan](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/lassoan/32/13_2.png) [@lassoan](https://discourse.slicer.org/u/lassoan)\
**Post date:** [May 24, 2023, 10:44am UTC](https://discourse.slicer.org/t/build-when-git-commit-is-changed/29607/5 "2023-05-24T10:44:59Z")

</div>

`libMRMLCore` is supposed to be just an import library that loads the shared library. If the public interface of the library does not change then the import library does not change and no re-linking should occur. That’s why it is very important to avoid including `vtkSlicerVersionConfigure.h` in any header file. Try removing the include in `qSlicerAbstractModule.h`, it might solve the excessive rebuild issue.

---

<div class="post-metadata">

**Author:** ![aymeric.chataigner](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/aymeric.chataigner/32/4977_2.png) [@aymeric.chataigner](https://discourse.slicer.org/u/aymeric.chataigner)\
**Post date:** [May 24, 2023, 1:07pm UTC](https://discourse.slicer.org/t/build-when-git-commit-is-changed/29607/6 "2023-05-24T13:07:23Z")

</div>

Many links are done simply if:

- I modify vtkMRMLVolumeSequenceStorageNode.cxx (simply add a space to a comment)
- launch build in Slicer-build (make -j12)  
A log screenshot is attached (I can provide the full log as a text file if you need it)

Removing vtkSlicerVersionConfigure.h in qSlicerAbstractModule.h is nice but it does not make significant change because my problem is really linking time of each library which depends on libraries modified by vtkSlicerVersionConfigure.h.

I would like to propose this improvement:

- use a resource file everywhere it is possible
- if information is required at build time: include vtkSlicerVersionConfigureInternal.h rather than vtkSlicerVersionConfigure.h  
 ![Screenshot from 2023-05-24 15-10-54](https://us1.discourse-cdn.com/flex002/uploads/slicer/original/3X/0/8/086f665ff1735f6f64d5c63bf535b070a87bb892.png)

---

<div class="post-metadata">

**Author:** ![lassoan](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/lassoan/32/13_2.png) [@lassoan](https://discourse.slicer.org/u/lassoan)\
**Post date:** [May 24, 2023, 2:33pm UTC](https://discourse.slicer.org/t/build-when-git-commit-is-changed/29607/7 "2023-05-24T14:33:33Z")

</div>

Removing references to vtkSlicerVersionConfigure.h in implementation (.cxx) files should not matter, as they should not trigger any relinking in other libraries. The .lib file is only modified when the public API (exported symbols in header files) is changed. When you change the content of a .cxx file then only the .so file is updated, which is not used during the linking (the content will only matter at runtime).

That said, I could reproduce the issue: touching `vtkMRMLVolumeSequenceStorageNode.cxx` and then running `make -j20` took 52 seconds (while without this change, make took 2 seconds).

@jcfr Do you know what can cause this unnecessary re-linking?

---

<div class="post-metadata">

**Author:** ![lassoan](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/lassoan/32/13_2.png) [@lassoan](https://discourse.slicer.org/u/lassoan)\
**Post date:** [May 25, 2023, 12:15am UTC](https://discourse.slicer.org/t/build-when-git-commit-is-changed/29607/8 "2023-05-25T00:15:19Z")

</div>

On Windows, the build works as it is supposed to. After modifying `vtkMRMLVolumeSequenceStorageNode.cxx` (not just whitespace change but actual implementation change) the build time is just 3 seconds longer. If no file is modified then build takes 14 seconds (due to Python hierarchy files are updated at every build); after modifying the storage node implementation, build takes just slightly longer - 17 seconds.

@pieper could you test it on macOS? How long `make -jN` takes for you on a non-modified build tree and how long does it take after you make a small change in `vtkMRMLVolumeSequenceStorageNode.cxx`?

---

<div class="post-metadata">

**Author:** ![lassoan](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/lassoan/32/13_2.png) [@lassoan](https://discourse.slicer.org/u/lassoan)\
**Post date:** [May 25, 2023, 1:18am UTC](https://discourse.slicer.org/t/build-when-git-commit-is-changed/29607/9 "2023-05-25T01:18:14Z")

</div>

@aymeric.chataigner The problem can be fixed by adding `set(CMAKE_LINK_DEPENDS_NO_SHARED TRUE)` to the top-level CMakeLists.txt (near the top, for example right below the C++ standard section)!

If only the implementation (`vtkMRMLVolumeSequenceStorageNode.cxx`) is changed then rebuild takes a couple of seconds. If the header file (public interface) is changed then the rebuild takes about 30 seconds.

There is a very [old discussion](https://cmake-developers.cmake.narkive.com/dTGZ68X4/should-cmake-link-depends-no-shared-be-on-by-default) about enabling this flag by default. Some people brought up some contrived examples and some tests failed (maybe just on Windows?), so at that time they did not enable the `CMAKE_LINK_DEPENDS_NO_SHARED` flag by default, and it seems that it is still disabled by default.

@jcfr do you think we should enable `CMAKE_LINK_DEPENDS_NO_SHARED` or we should set target linking properties more carefully to prevent the unnecessary relinking on linux?

---

<div class="post-metadata">

**Author:** ![jcfr](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/jcfr/32/17825_2.png) [@jcfr](https://discourse.slicer.org/u/jcfr)\
**Post date:** [May 25, 2023, 1:21am UTC](https://discourse.slicer.org/t/build-when-git-commit-is-changed/29607/10 "2023-05-25T01:21:18Z")

</div>

> […]`vtkSlicerVersionConfigure.h` […] in `qSlicerAbstractModule.h` […] could be probably removed,

Indeed, this was added back in 2012 in the context of [Slicer@b6aab58a2](https://github.com/Slicer/Slicer/commit/b6aab58a26890b7cba929ce4a8917e51bcae9d1e).

Since then, in mid-2021, the approach has been revisited in [Slicer@ae2e56b49](https://github.com/Slicer/Slicer/commit/ae2e56b493138eaa9d9082951f49bb3d693a2a9d) and there is no need have macro `Slicer_VERSION_MAJOR` and `Slicer_VERSION_MINOR` defined for module to compile.

Removing the include of `vtkSlicerVersionConfigure.h` in `qSlicerAbstractModule.h` should not be an issue for Slicer core.

And while it may affect some extension, it will be easy to fix.

---

<div class="post-metadata">

**Author:** ![jcfr](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/jcfr/32/17825_2.png) [@jcfr](https://discourse.slicer.org/u/jcfr)\
**Post date:** [May 25, 2023, 1:31am UTC](https://discourse.slicer.org/t/build-when-git-commit-is-changed/29607/11 "2023-05-25T01:31:55Z")

</div>

> [@lassoan](#):
>
> should set target linking properties more carefully to prevent the unnecessary relinking on linux?

Ideally yes, this will happen organically as we start transitioning the build system to fully adopt the modern cmake paradigm where the usage requirements are all associated with our targets. See [here](https://cmake.org/cmake/help/latest/manual/cmake-buildsystem.7.html#build-specification-and-usage-requirements)

> [@lassoan](#):
>
> should enable `CMAKE_LINK_DEPENDS_NO_SHARED`

In the meantime, enabling this option to improve the develop experience seems reasonable.

I just sent a quick note to our Kitware CMake channel to ask if there any caveats folks may be aware of.

---

<div class="post-metadata">

**Author:** ![jcfr](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/jcfr/32/17825_2.png) [@jcfr](https://discourse.slicer.org/u/jcfr)\
**Post date:** [May 25, 2023, 1:52am UTC](https://discourse.slicer.org/t/build-when-git-commit-is-changed/29607/12 "2023-05-25T01:52:19Z")

</div>

Once the inner-build has been reconfigured with `CMAKE_LINK_DEPENDS_NO_SHARED` set to `ON` and at least built once.

After modifying one implementation file

```auto
$ touch ../../Slicer/Libs/MRML/Core/vtkMRMLScene.cxx 

```

We can observe (on Linux) that beside the rebuilding of `libMRMLCore.so` no relinking happen for any of the dependencies:

```auto
$ make -j10 
[...]
[24%] Building CXX object Libs/MRML/Core/CMakeFiles/MRMLCore.dir/vtkMRMLScene.cxx.o
[24%] Linking CXX shared library ../../../bin/libMRMLCore.so
[26%] Built target MRMLCore
[26%] Built target MRMLCLI
[...]
[98%] Built target qSlicerTablesModuleCxxTests
[99%] Built target qSlicerTablesModuleGenericCxxTests
[99%] Built target qSlicerMarkupsModuleCxxTests
[99%] Built target qSlicerVolumeRenderingModuleGenericCxxTests
[100%] Built target qSlicerVolumeRenderingModuleCxxTests
```

---

<div class="post-metadata">

**Author:** ![aymeric.chataigner](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/aymeric.chataigner/32/4977_2.png) [@aymeric.chataigner](https://discourse.slicer.org/u/aymeric.chataigner)\
**Post date:** [May 25, 2023, 9:56am UTC](https://discourse.slicer.org/t/build-when-git-commit-is-changed/29607/13 "2023-05-25T09:56:08Z")

</div>

Thanks a lot for your help.

I set CMAKE\_LINK\_DEPENDS\_NO\_SHARED to ON and I removed #include “vtkSlicerVersionConfigure.h” in qSlicerAbstractModule

Now if I build after a change in vtkSlicerVersionConfigure.h (like it is done by cmake when my git branch goes onto the next commit which just changes README.md) it takes 2min30s rather than 8min.

Should I propose a Pull Request for this simple changes ?

Naturally It would be nicer if a simple commit change should cause NO compilation, NO link.

As I said before, could I propose a pull request which:

- use a resource file everywhere it is possible
- if information is required at build time: include vtkSlicerVersionConfigureInternal.h rather than vtkSlicerVersionConfigure.h if it is possible

---

<div class="post-metadata">

**Author:** ![pieper](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/pieper/32/8_2.png) [@pieper](https://discourse.slicer.org/u/pieper)\
**Post date:** [May 25, 2023, 1:31pm UTC](https://discourse.slicer.org/t/build-when-git-commit-is-changed/29607/14 "2023-05-25T13:31:13Z")

</div>

With current source, no cmake changes on Mac pro. Rebuilding Slicer-build with make -j50:

No changes: 8 seconds

Touching `vtkMRMLVolumeSequenceStorageNode.cxx`: 45 seconds (lots of relinking).

---

<div class="post-metadata">

**Author:** ![lassoan](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/lassoan/32/13_2.png) [@lassoan](https://discourse.slicer.org/u/lassoan)\
**Post date:** [May 25, 2023, 3:09pm UTC](https://discourse.slicer.org/t/build-when-git-commit-is-changed/29607/15 "2023-05-25T15:09:10Z")

</div>

@jcfr @pieper thanks for testing!

@aymeric.chataigner Can you confirm that building after touching `vtkMRMLVolumeSequenceStorageNode.cxx` just takes a couple of seconds now? That means that we solved a large part of the problem!

> [@aymeric.chataigner](#):
>
> As I said before, could I propose a pull request which:
> 
> - use a resource file everywhere it is possible
> - if information is required at build time: include vtkSlicerVersionConfigureInternal.h rather than vtkSlicerVersionConfigure.h if it is possible

Cleaning up `vtkSlicerVersionConfigure.h` includes would be nice, it would be great if you could work on it. I think there are a couple of places where it is simply not needed.

Using a resource file will probably not be necessary, because it is simpler to get version information at runtime using qSlicerCoreApplication methods.

As its name (and comment in the file header) suggests, `vtkSlicerVersionConfigureInternal.h` is for internal use - only to be included in `vtkSlicerVersionConfigure.h` and not to be used in modules. We could rename and repurpose this .h file, but I’m not sure if it is worth it (if it can lead to significant build time reduction). What classes you would replace the include with the internal?

---

<div class="post-metadata">

**Author:** ![aymeric.chataigner](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/aymeric.chataigner/32/4977_2.png) [@aymeric.chataigner](https://discourse.slicer.org/u/aymeric.chataigner)\
**Post date:** [May 25, 2023, 4:05pm UTC](https://discourse.slicer.org/t/build-when-git-commit-is-changed/29607/16 "2023-05-25T16:05:49Z")

</div>

> [@lassoan](#):
>
> Can you confirm that building after touching `vtkMRMLVolumeSequenceStorageNode.cxx` just takes a couple of seconds now?

Yes !

> [@lassoan](#):
>
> As its name (and comment in the file header) suggests, `vtkSlicerVersionConfigureInternal.h` is for internal use - only to be included in `vtkSlicerVersionConfigure.h` and not to be used in modules. We could rename and repurpose this .h file, but I’m not sure if it is worth it (if it can lead to significant build time reduction). What classes you would replace the include with the internal?

In my wip branch I replaced it in:  
vtkMRMLVolumeSequenceStorageNode.cxx  
vtkMRMLMarkupsFiducialStorageNode.cxx  
qSlicerApplicationHelper.cxx

I will try to continue tomorrow and I will let you know.

---

<div class="post-metadata">

**Author:** ![lassoan](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/lassoan/32/13_2.png) [@lassoan](https://discourse.slicer.org/u/lassoan)\
**Post date:** [May 25, 2023, 5:09pm UTC](https://discourse.slicer.org/t/build-when-git-commit-is-changed/29607/17 "2023-05-25T17:09:50Z")

</div>

vtkMRMLVolumeSequenceStorageNode → The version check can be removed and we can just use the code snippet in the `Slicer_VERSION_MAJOR > 4` branch.

vtkMRMLMarkupsFiducialStorageNode → it uses `Slicer_VERSION`, which is not available in vtkSlicerVersionConfigureInternal.h, so I think we need to keep using vtkSlicerVersionConfigure.h. It is just an implementation file in a small library, so recompiling&linking should not take more than a few seconds anyway.

qSlicerApplicationHelper → it uses `Slicer_MAIN_PROJECT_VERSION_FULL`, which is not available in vtkSlicerVersionConfigureInternal.h, so I think we need to keep using vtkSlicerVersionConfigure.h. If you find the recompilation of this file makes a huge difference then we can have a look how to avoid it.

---

<div class="post-metadata">

**Author:** ![aymeric.chataigner](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/aymeric.chataigner/32/4977_2.png) [@aymeric.chataigner](https://discourse.slicer.org/u/aymeric.chataigner)\
**Post date:** [May 30, 2023, 2:18pm UTC](https://discourse.slicer.org/t/build-when-git-commit-is-changed/29607/18 "2023-05-30T14:18:00Z")

</div>

Thanks again for [COMP: Speed up incremental build on Linux by achataigner · Pull Request #6983 · Slicer/Slicer · GitHub](https://github.com/Slicer/Slicer/pull/6983#issuecomment-1567916267)

I also had to do that in SlicerRT: [COMP: remove use of constants defined in vtkSlicerVersionConfigure.h by achataigner · Pull Request #230 · SlicerRt/SlicerRT · GitHub](https://github.com/SlicerRt/SlicerRT/pull/230)

---

<div class="post-metadata">

**Author:** ![jamesobutler](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/jamesobutler/32/7511_2.png) [@jamesobutler](https://discourse.slicer.org/u/jamesobutler)\
**Post date:** [May 31, 2023, 10:12pm UTC](https://discourse.slicer.org/t/build-when-git-commit-is-changed/29607/19 "2023-05-31T22:12:28Z")

</div>

> [@jcfr](#):
>
> And while it may affect some extension, it will be easy to fix.

- SlicerRT [build error](https://slicer.cdash.org/viewBuildError.php?buildid=3045350)
  - Fix → [COMP: remove use of constants defined in vtkSlicerVersionConfigure.h by achataigner · Pull Request #230 · SlicerRt/SlicerRT · GitHub](https://github.com/SlicerRt/SlicerRT/pull/230)

- SlicerDMRI [build error](https://slicer.cdash.org/viewBuildError.php?buildid=3045378)
- Chest\_Imaging\_Platform [build error](https://slicer.cdash.org/viewBuildError.php?buildid=3045361)

---

<div class="post-metadata">

**Author:** ![aymeric.chataigner](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/aymeric.chataigner/32/4977_2.png) [@aymeric.chataigner](https://discourse.slicer.org/u/aymeric.chataigner)\
**Post date:** [June 5, 2023, 9:51am UTC](https://discourse.slicer.org/t/build-when-git-commit-is-changed/29607/20 "2023-06-05T09:51:19Z")

</div>

I proposed these PR to continue to save more time during incremetal build:

> <https://github.com/Slicer/Slicer/pull/7001>
>
> vtkSlicerVersionConfigureInternal.h is renamed vtkSlicerVersionConfigureMinimal.…h.
> 
> \#include "vtkSlicerVersionConfigure.h" is replaced by #include "vtkSlicerVersionConfigureMinimal.h" when it is possible, therefore no compilation is required if git commit is changed.

> <https://github.com/SlicerRt/SlicerRT/pull/231>
>
> Apply required changes from https://github.com/Slicer/Slicer/pull/7001

On my PC with ubuntu 20.04 running an intel i7 6 hyperthreaded cores:  
If I do make -j12:  
Build slicer incremental: 35 seconds rather than 40 seconds  
Build slicerRT incremental: 30 seconds rather than 1 minute

[Next page](https://discourse.slicer.org/t/build-when-git-commit-is-changed/29607.md?page=2)
