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

module/Makefile.in: don't run xargs if empty #9418

Merged
merged 1 commit into from
Oct 8, 2019

Conversation

gmelikov
Copy link
Member

@gmelikov gmelikov commented Oct 6, 2019

Motivation and Context

If stdin is empty - don't run xargs command,
otherwise we can get cp: missing file operand
error (reproducer triggers on spl dir).

How Has This Been Tested?

Reproducer - 'make -j 2 pkg-utils deb-kmod'

list='avl/ icp/ lua/ nvpair/ spl/ os/linux/spl/ unicode/ zcommon/ zfs/ os/linux/zfs/'; for objdir in $list; do \
	(cd ../module && find $objdir \
	-name '*.c' -o -name '*.h' -o -name '*.S' | \
	xargs cp --parents -t /home/travis/build/gmelikov/zfs/module/$distdir); \
done

cp: missing file operand

Don't reproduce after this patch.

Types of changes

  • 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)
  • Documentation (a change to man pages or other documentation)

Checklist:

If stdin if empty - don't run xargs command,
otherwise we can get `cp: missing file operand`
error.

Signed-off-by: George Melikov <mail@gmelikov.ru>
@gmelikov
Copy link
Member Author

gmelikov commented Oct 6, 2019

Hmm, looks like now we have some problems with make -j X with X >1. I've got another error with it.

@codecov
Copy link

codecov bot commented Oct 6, 2019

Codecov Report

Merging #9418 into master will increase coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9418      +/-   ##
==========================================
+ Coverage   78.95%   79.02%   +0.07%     
==========================================
  Files         410      410              
  Lines      122499   122499              
==========================================
+ Hits        96723    96809      +86     
+ Misses      25776    25690      -86
Flag Coverage Δ
#kernel 79.74% <ø> (-0.01%) ⬇️
#user 66.8% <ø> (+0.58%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94bcf6f...f48fbfa. Read the comment docs.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

This will be fine for FreeBSD.

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Oct 7, 2019
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.

Looks good, just one portability question.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Oct 7, 2019
@behlendorf
Copy link
Contributor

looks like now we have some problems with make -j X with X >1. I've got another error with it.

I don't think I've seen this. What's the error?

@gmelikov
Copy link
Member Author

gmelikov commented Oct 7, 2019

@behlendorf

# make -j 2 pkg-utils deb-kmod > out.log 2> out2.log
# echo $?
2
# cat out2.log
cp: cannot create regular file 'zfs-0.8.0/config/ax_python_devel.m4': File exists
make[3]: *** [Makefile:965: distdir-am] Error 1
make[2]: *** [Makefile:960: distdir] Error 2
make[1]: *** [Makefile:1065: dist] Error 2
make: *** [Makefile:1302: srpm-utils] Error 2
make: *** Waiting for unfinished jobs....

Looks like now each thread tries to do the same thing with ./zfs-0.8.0. Maybe there is a better fix, but the fast one is to ignore such cases.

@behlendorf
Copy link
Contributor

@gmelikov the -j option is not needed when building packages. The spec files internally use %{?_smp_mflags} will set -j appropriately for the system. Though, I agree it shouldn't result in an error.

@behlendorf behlendorf merged commit 7b50929 into openzfs:master Oct 8, 2019
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Dec 26, 2019
If stdin if empty - don't run xargs command,
otherwise we can get `cp: missing file operand`
error.

Reviewed-by: Ryan Moeller <ryan@ixsystems.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: George Melikov <mail@gmelikov.ru>
Closes openzfs#9418
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Dec 27, 2019
If stdin if empty - don't run xargs command,
otherwise we can get `cp: missing file operand`
error.

Reviewed-by: Ryan Moeller <ryan@ixsystems.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: George Melikov <mail@gmelikov.ru>
Closes openzfs#9418
tonyhutter pushed a commit that referenced this pull request Jan 23, 2020
If stdin if empty - don't run xargs command,
otherwise we can get `cp: missing file operand`
error.

Reviewed-by: Ryan Moeller <ryan@ixsystems.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: George Melikov <mail@gmelikov.ru>
Closes #9418
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.

2 participants