Skip to content

dist: add missing FindNettle.cmake#14320

Closed
vszakats wants to merge 1 commit intocurl:masterfrom
vszakats:distmiss
Closed

dist: add missing FindNettle.cmake#14320
vszakats wants to merge 1 commit intocurl:masterfrom
vszakats:distmiss

Conversation

@vszakats
Copy link
Member

@vszakats vszakats commented Jul 31, 2024

Follow-up to 669ce42 #14285
Reported-by: Christoph Reiter
Bug: #14285 (comment)
Closes #14320

@vszakats vszakats changed the title dist: add missing FindNettle.cmake dist: add missing FindNettle.cmake Jul 31, 2024
@github-actions github-actions bot added the build label Jul 31, 2024
@vszakats
Copy link
Member Author

vszakats commented Jul 31, 2024

I wonder how this could be caught in CI. There is a test, but I guess we'd need separate ones to touch each Find* module, which is then just as easy to forget about as adding them to /Makefile.am.

@bagder
Copy link
Member

bagder commented Jul 31, 2024

Unless we can write a parser that figures out what modules we use and then check that they are all present...

@vszakats vszakats mentioned this pull request Jul 31, 2024
@talregev
Copy link
Contributor

You can create test that curl compile with other library/program (curl is a 3rd party) and check this things.

@talregev
Copy link
Contributor

Similar to test, but test have different cmake that don't depends on curl cmake.

@vszakats
Copy link
Member Author

Here's an attempt to test this in CI (and locally with the -DENABLE_DIST_TEST=ON option): #14323

Follow-up to 669ce42 curl#14285
Reported-by: Christoph Reiter
Bug: curl#14285 (comment)
Closes curl#14320
@bagder
Copy link
Member

bagder commented Jul 31, 2024

You can create test that curl compile with other library/program (curl is a 3rd party) and check this things.

sure, but then we would need to make builds (from a dist tarball) for all dependencies that use a cmake module

@talregev
Copy link
Contributor

talregev commented Jul 31, 2024

Maybe the bug can reproduce without the distro tar ball, with simple make install for curl and install dependencies with apt get or similar. Write other cmake that will try to find curl and it dependencies.

@vszakats
Copy link
Member Author

The tarball or building anything is unnecessary. I went with a CMake custom option that reads Makefile.am and checks if all on-disk .cmake/.in files are referenced from it.

@talregev
Copy link
Contributor

talregev commented Jul 31, 2024

I think we talking here about more errors that can be found with cmake builds and make them automatic.

@vszakats vszakats closed this in 58946ee Jul 31, 2024
@vszakats vszakats deleted the distmiss branch July 31, 2024 14:34
vszakats added a commit to vszakats/curl that referenced this pull request Jul 31, 2024
- add CMake option to verify if the `CMake/*.cmake`, `CMake/*.in` files
  are listed as distributable in autotools' `EXTRA_DIST`. The check can
  be enabled with `-DENABLE_DIST_TEST=ON` CMake option.

- add CI job to that effect.

Ref: curl#14320
Closes curl#14323
vszakats added a commit that referenced this pull request Jul 31, 2024
- add CMake option to verify if the `CMake/*.cmake`, `CMake/*.in` files
  are listed as distributable in autotools' `EXTRA_DIST`. The check can
  be enabled with `-DENABLE_DIST_TEST=ON` CMake option.

- add CI job to that effect.

Ref: #14320
Closes #14323
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants