Skip to content

Conversation

ellert
Copy link
Contributor

@ellert ellert commented Dec 19, 2024

This Pull request:

Changes or fixes:

Using the -DCLAD_SOURCE_DIR currently does not disable the git checkout. From the cmake documentation for ExternalProject_Add:

Note: If a download method is specified, any existing contents of the source directory may be deleted. Only the URL download method checks whether this directory is either missing or empty before initiating the download, stopping with an error if it is not empty. All other download methods silently discard any previous contents of the source directory.

Currently cmake deletes the source directory defined by -DCLAD_SOURCE_DIR and then tries to recreate it using a git checkout, but fails if there is no network available:

Cloning into 'clad-1.7'...
fatal: unable to access 'https://github.com/vgvassilev/clad.git/': Could not resolve host: github.com Cloning into 'clad-1.7'...
fatal: unable to access 'https://github.com/vgvassilev/clad.git/': Could not resolve host: github.com Cloning into 'clad-1.7'...
fatal: unable to access 'https://github.com/vgvassilev/clad.git/': Could not resolve host: github.com Had to git clone more than once: 3 times.
CMake Error at redhat-linux-build/interpreter/cling/tools/plugins/clad/clad-prefix/tmp/clad-gitclone.cmake:50 (message):
Failed to clone repository: 'https://github.com/vgvassilev/clad.git'

The attempt to disable the git checkout by setting the DOWNLOAD_COMMAND option to an empty string is not working.

This commit fixes the issue by only defining the GIT_REPOSITORY and GIT_TAG options when CLAD_SOURCE_DIR is not defined.

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

Using the -DCLAD_SOURCE_DIR currently does not disable the git checkout.
From the cmake documentation for ExternalProject_Add:

Note: If a download method is specified, any existing contents of the
source directory may be deleted. Only the URL download method checks
whether this directory is either missing or empty before initiating
the download, stopping with an error if it is not empty. All other
download methods silently discard any previous contents of the source
directory.

Currently cmake deletes the source directory defined by
-DCLAD_SOURCE_DIR and then tries to recreate it using a git checkout,
but fails if there is no network available:

Cloning into 'clad-1.7'...
fatal: unable to access 'https://github.com/vgvassilev/clad.git/': Could not resolve host: github.com
Cloning into 'clad-1.7'...
fatal: unable to access 'https://github.com/vgvassilev/clad.git/': Could not resolve host: github.com
Cloning into 'clad-1.7'...
fatal: unable to access 'https://github.com/vgvassilev/clad.git/': Could not resolve host: github.com
Had to git clone more than once: 3 times.
CMake Error at redhat-linux-build/interpreter/cling/tools/plugins/clad/clad-prefix/tmp/clad-gitclone.cmake:50 (message):
  Failed to clone repository: 'https://github.com/vgvassilev/clad.git'

The attempt to disable the git checkout by setting the
DOWNLOAD_COMMAND option to an empty string is not working.

This commit fixes the issue by only defining the GIT_REPOSITORY and
GIT_TAG options when CLAD_SOURCE_DIR is not defined.
Copy link
Member

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me! Thank you!

Copy link
Member

@bellenot bellenot left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@bellenot bellenot merged commit 0e68370 into root-project:master Dec 19, 2024
18 of 20 checks passed
@ellert ellert deleted the clad-src-dir branch December 20, 2024 11:51
@guitargeek
Copy link
Contributor

Awesome, thanks for fixing this up! My bad, I didn't notice then that it didn't work because I always had internet connection.

guitargeek added a commit to guitargeek/nixpkgs that referenced this pull request Feb 10, 2025
Also enables one more feature of ROOT (support of convolutions via
fftw). Furthermore removes a patch related to building Clad from source
that is not needed anymore thanks to
root-project/root#17308.
guitargeek added a commit to guitargeek/nixpkgs that referenced this pull request Feb 14, 2025
Also enables one more feature of ROOT (support of convolutions via
fftw). Furthermore removes a patch related to building Clad from source
that is not needed anymore thanks to
root-project/root#17308.
guitargeek added a commit to guitargeek/nixpkgs that referenced this pull request Feb 14, 2025
Also enables one more feature of ROOT (support of convolutions via
fftw). Furthermore removes a patch related to building Clad from source
that is not needed anymore thanks to
root-project/root#17308.
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