Skip to content
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

fix make distcheck problem in dap4_test (but turning off nczarr_test/run_scalar.sh due to issue) #2546

Closed
wants to merge 10 commits into from

Conversation

edwardhartnett
Copy link
Contributor

@edwardhartnett edwardhartnett commented Nov 4, 2022

Fixes #2544
Fixes #2547
Fixes #2548

Had to turn off nczarr_test/run_scalar.sh

Also turn on distcheck target in CI build.

See #2545

@edwardhartnett edwardhartnett requested a review from WardF as a code owner November 4, 2022 14:07
@edwardhartnett
Copy link
Contributor Author

When I run this branch through my Jenkins server it builds well for all supported versions of HDF5, with and without zstd, and with and without szip. So that's good!

Parallel testing next...

@@ -181,14 +181,9 @@ jobs:
run: CFLAGS=${CFLAGS} LDFLAGS=${LDFLAGS} LD_LIBRARY_PATH=${LD_LIBRARY_PATH} make -j
if: ${{ success() }}

- name: Build Tests
Copy link
Member

Choose a reason for hiding this comment

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

The addition of the DISTCHECK_CONFIGURE_FLAGS is great information, thanks! We'll want to keep the organization we have where build tests and run tests are separate steps in the workflow, for consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well I'm not sure how that can be done with the distcheck target.

The distcheck target makes a dist, unpacks it, builds it (outside the source directory) and runs make check, all in one step.

So I don't know how we can build tests in one step and run them in another...

Copy link
Contributor

@DWesl DWesl Nov 12, 2022

Choose a reason for hiding this comment

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

The main use of the distcheck target over the check and installcheck targets, in my understanding, is checking whether the project

  1. Builds from a source distribution, not just from a repository
  2. Can perform behaviors expected of Autotools packages, namely an out-of-tree build.
  3. uninstalls and cleans up properly (make uninstall and make distclean).

If you can add a single run of make distcheck that only runs if all the Autotools runs of make check succeed, that should complete all objectives.

Another way to complete the second objective within the current framework is to perform all of the build and test steps in a subdirectory (i.e., start the configure step with mkdir build; cd build, and start every subsequent step with cd build).

It would also be possible to start the Linux Autotools runs with make dist, upload the resulting distributions, and use those instead of actions/checkout for getting the source code in later steps, which would achieve the first objective.

The make distclean check is easy with the out-of-tree build (if there's any files there, it's not in the state it was before ../configure) and should be straightforward with an in-repository build (fiddle with git status and friends until it tells you if there are any files not checked in, then check that the only new files are configure, Makefile.in, and friends).

@edwardhartnett
Copy link
Contributor Author

OK, has this all been fixed in other ways?

SHould I just close this PR?

@WardF
Copy link
Member

WardF commented Nov 30, 2022

@edwardhartnett I believe it can be closed, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants