Skip to content

CI: add autotools, out-of-tree, debug build to distro check job#12088

Closed
vszakats wants to merge 11 commits intocurl:masterfrom
vszakats:ci-test-distcheck
Closed

CI: add autotools, out-of-tree, debug build to distro check job#12088
vszakats wants to merge 11 commits intocurl:masterfrom
vszakats:ci-test-distcheck

Conversation

@vszakats
Copy link
Member

@vszakats vszakats commented Oct 11, 2023 •
edited
Loading

Add a job that builds curl from a generated source tarball sample, with
autotools, out-of-tree, in debug mode.

Ref: #12085
Closes #12088

@vszakats vszakats marked this pull request as draft October 11, 2023 09:09
@github-actions github-actions bot added the CI Continuous Integration label Oct 11, 2023
@vszakats vszakats mentioned this pull request Oct 11, 2023
Closed
@vszakats
Copy link
Member Author

vszakats commented Oct 11, 2023 

make[3]: *** No rule to make target 'Makefile.dist', needed by 'distdir-am'.  Stop.

https://github.com/curl/curl/actions/runs/6480792976/job/17597019192#step:7:3863

@vszakats
Copy link
Member Author

vszakats commented Oct 11, 2023

Stuck at:

make[3]: *** No rule to make target 'Makefile.dist', needed by 'distdir-am'.  Stop.

https://github.com/curl/curl/actions/runs/6480934827/job/17597469241#step:7:2658

@vszakats vszakats changed the title TESTING: ci test distcheck ci: try adding make distcheck Oct 11, 2023
@vszakats
Copy link
Member Author

vszakats commented Oct 11, 2023 •
edited
Loading

This was "easy": Makefile.dist is referenced from Makefile.am in EXTRA_DIST, while apparently it shouldn't be there. First because distcheck breaks on this, second because this file isn't part of the of the source tarball (not with this filename anyway).

Next problem: ERROR: files left in build directory after distclean: followed by a list of files created during the build process:
https://github.com/curl/curl/actions/runs/6483154400/job/17604109905#step:7:4028

Next-next problem: distcheck actually doesn't complain about a missing src/.checksrc at all. Perhaps this isn't even what it's meant to do, though I swear to have been warned about missing files in e.g. libssh2, when running distcheck.

Running it with --enable-debug makes it fail without that particular file, but distcheck isn't needed for that, and autotools debug version of verify-out-of-tree-cmake would do the job (not tested yet).

@vszakats
Copy link
Member Author

vszakats commented Oct 11, 2023

Dropped the make distcheck idea, though in theory it's probably the way to fix this class of problem.
Added a debug out-of-tree configure job instead.

Example of it catching the missing src/.checksrc:
https://github.com/curl/curl/actions/runs/6483454571/job/17605256636#step:3:4907

@vszakats vszakats changed the title ci: try adding make distcheck ci: add autotools + out-of-tree + debug build to distro check job Oct 11, 2023
@vszakats vszakats requested review from bagder and mback2k October 11, 2023 14:16
@vszakats vszakats marked this pull request as ready for review October 11, 2023 14:16
@vszakats
Copy link
Member Author

vszakats commented Oct 11, 2023 •
edited
Loading

Now it's this:

/usr/bin/install: cannot create regular file '/usr/local/lib/libcurl.so.4.8.0': Permission denied

I'm giving up!

@vszakats
Copy link
Member Author

vszakats commented Oct 11, 2023

Okay, let's try without install first...

@bagder
Copy link
Member

bagder commented Oct 11, 2023 •
edited
Loading

because distcheck breaks on this, second because this file isn't part of the of the source tarball (not with this filename anyway).

make dist makes all *.dist files get included in the tarball without the .dist extension. So the Makefile.dist file ends up as just Makefile in the tarball.

@vszakats
Copy link
Member Author

vszakats commented Oct 11, 2023

Phew, green now:
https://github.com/curl/curl/actions/runs/6483854257/job/17606626203?pr=12088

This is already rebased on master with the src/.checksrc fix.

@vszakats
Copy link
Member Author

vszakats commented Oct 11, 2023

@bagder: Do you think we should delete Makefile.dist off the EXTRA_DIST list, or should is stay there? (this patch deletes it at the moment)

@dfandrich
Copy link
Contributor

dfandrich commented Oct 11, 2023 via email

@vszakats
Copy link
Member Author

vszakats commented Oct 11, 2023

That did the trick, thanks Dan!

@bagder
Copy link
Member

bagder commented Oct 12, 2023

@bagder: Do you think we should delete Makefile.dist off the EXTRA_DIST list, or should is stay there? (this patch deletes it at the moment)

I believe the reasoning for including Makefile.dist in the tarball is that it lets a user make a subsequent release with maketgz based on only the files provided in the tarball, and I believe that is a sensible policy. This, since the root Makefile might get overwritten if for example configure is invoked from within the source directory.

So yes: I think it can remain in EXTRA_DIST.

@vszakats
Copy link
Member Author

vszakats commented Oct 12, 2023 •
edited
Loading

@bagder: Thanks, I'll revert this, but Makefile.dist is not present in the release tarball (I'm guessing due to the error showcased above), which might need another fix.

@bagder
Copy link
Member

bagder commented Oct 12, 2023

@bagder: Thanks, I'll revert this, but Makefile.dist is not present in the release tarball (I'm guessing due to the error showcased above), which might need another fix.

Ah yes, and that's a slightly tricky one since we have that hook that puts .dist files into the tarball without that extension:

curl/Makefile.am

Lines 171 to 178 in a20d7bd

dist-hook:
rm -rf $(top_builddir)/tests/log
find $(distdir) -name "*.dist" -exec rm {} \;
(distit=`find $(srcdir) -name "*.dist" | grep -v ./ares/`; \
for file in $$distit; do \
strip=`echo $$file | sed -e s/^$(srcdir)// -e s/\.dist//`; \
cp -p $$file $(distdir)$$strip; \
done)

@vszakats vszakats changed the title ci: add autotools + out-of-tree + debug build to distro check job CI: add autotools, out-of-tree, debug build to distro check job Oct 12, 2023
@vszakats vszakats closed this in 2e283c6 Oct 12, 2023
@vszakats vszakats deleted the ci-test-distcheck branch October 12, 2023 23:31
@thesamesam thesamesam mentioned this pull request Mar 8, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mback2k mback2k Awaiting requested review from mback2k

@bagder bagder Awaiting requested review from bagder

Assignees

No one assigned

Labels

CI Continuous Integration tests

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vszakats @bagder @dfandrich