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

Add Makefile dependencies for scripts/zfs-tests.sh -c #16922

Merged
merged 1 commit into from
Jan 4, 2025

Conversation

rrevans
Copy link
Contributor

@rrevans rrevans commented Jan 3, 2025

This updates the Makefile to be more correct for parallel make.

Adding explicit dependencies forces make to build all outputs before running the script to populate tests/zfs-tests/bin.

Really this target only needs to depend on targets used by tests/zfs-tests/include/commands.cfg, but waiting for primary automake targets is easier and more straightforward.

Without this, parallel make might install symlinks before all outputs are built. This is problematic when also doing sudo make install afterward as that step repeats this step causing some files to end up owned by root. That ultimately confuses ZTS into adding non-root test users to the root group causing test breakage.

Fixes #16030

Motivation and Context

Using make -j32 causes flakiness when also doing sudo make install before running ZTS.

See the debugging quest at #16030 for the full story, but also the Makefile should be correct for parallel builds.

Description

Adds the automake targets PROGRAMS, DATA, and SCRIPTS to scripts-all-local to force this step to run after all of the primary targets are built.

How Has This Been Tested?

Ran make -j32 after make distclean and observed that scripts/zfs-tests.sh -c ran after primary targets as expected.

Types of changes

This is a Makefile-only change.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

This updates the Makefile to be more correct for parallel make.

Fixes openzfs#16030

Signed-off-by: Robert Evans <[email protected]>
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Thanks for the clear walk through in #16030 about exactly how this can happen. That makes perfect sense, as does the fix to add the required missing dependencies. Nice job getting to the bottom of this!

@behlendorf behlendorf added the Status: Accepted Ready to integrate (reviewed, tested) label Jan 3, 2025
@behlendorf behlendorf merged commit 50cbb14 into openzfs:master Jan 4, 2025
23 of 25 checks passed
behlendorf pushed a commit to behlendorf/zfs that referenced this pull request Jan 4, 2025
This updates the Makefile to be more correct for parallel make.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: George Melikov <[email protected]>
Signed-off-by: Robert Evans <[email protected]>
Closes openzfs#16030
Closes openzfs#16922
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ZTS: acl/off/posixmode never passes?
3 participants