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

Minor cleanups for Debian packaging (lintian warnings and errors) #8803

Closed
wants to merge 2 commits into from

Conversation

siv0
Copy link
Contributor

@siv0 siv0 commented May 23, 2019

Motivation and Context

The 2 commits fix cosmetic issues I encountered while packaging zfs-0.8.0-rc5 for PVE/Debian.

Description

  • the shebang of tests/test-runner/bin/test-runner.py was changed from /usr/bin/python to /usr/bin/python3 - the buildsystem replaces this with python2, if it is the version used/found during ./configure
  • .shlib, .kshlib and .cfg files listed in dist_pkgdata_SCRIPTS were instead put into dist_pkgdata_DATA in order to be installed 0644 instead of 0755
  • the shebang was removed from one .kshlib file.

How Has This Been Tested?

  • the zfs-test package was build on Debian and run through lintian, which produced fewer warnings with those patches
  • additionally make rpm-build was run on a fedora 30 installation - once with python3 only installed and once after running ./configure --with-python=2 - I checked the resulting files in the rpms

Types of changes

  • [ x] 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:

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label May 23, 2019
@codecov
Copy link

codecov bot commented May 24, 2019

Codecov Report

Merging #8803 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8803      +/-   ##
==========================================
- Coverage   78.73%   78.71%   -0.02%     
==========================================
  Files         382      382              
  Lines      117812   117812              
==========================================
- Hits        92758    92740      -18     
- Misses      25054    25072      +18
Flag Coverage Δ
#kernel 79.24% <ø> (-0.1%) ⬇️
#user 67.31% <ø> (+0.05%) ⬆️

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 841a7a9...e2390b7. Read the comment docs.

@behlendorf behlendorf requested review from loli10K and jwk404 May 24, 2019 21:22
@@ -1,4 +1,4 @@
#!/usr/bin/python
#!/usr/bin/python3
Copy link
Contributor

Choose a reason for hiding this comment

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

This commit changes test-runner.py to also use /usr/bin/python3,
enabling the change during buildtime and fixing a minor lintian issue
on Debian systems, which only have python3 installed.

The highlighted part of the commit message seems to suggest (all) Debian systems don't provide python2. I tested this change on a Debian8 (Python 2.7.9) but the number of lintian warning/errors regarding test-runner.py seems to be the same as the unpatched version:

root@linux:~# lsb_release -a
No LSB modules are available.
Distributor ID:	Debian
Description:	Debian GNU/Linux 8.0 (jessie)
Release:	8.0
Codename:	jessie
root@linux:~# python -V
Python 2.7.9
root@linux:~# grep runner /var/tmp/patched /var/tmp/vanilla 
/var/tmp/patched:E: zfs-test: python-script-but-no-python-dep usr/share/zfs/test-runner/bin/test-runner.py
/var/tmp/patched:E: zfs-test: python-script-but-no-python-dep usr/share/zfs/test-runner/bin/zts-report.py
/var/tmp/vanilla:E: zfs-test: python-script-but-no-python-dep usr/share/zfs/test-runner/bin/test-runner.py
/var/tmp/vanilla:E: zfs-test: python-script-but-no-python-dep usr/share/zfs/test-runner/bin/zts-report.py
root@linux:~# 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review and for catching the wrong phrasing of the commit message!
I just pushed a reworded version.

Additionally tested by building zfs while explicitly configuring for python2, and adapting the appropriate fields in the debian/control file - The lintian errors don't appear, and the shebang of test-runner.py is set to /usr/bin/python2

Stoiko Ivanov added 2 commits May 27, 2019 13:34
In commit 6e72a5b python scripts which
work with python2 and python3 changed the shebang from /usr/bin/python
to /usr/bin/python3. This gets adapted by the build-system on systems
which don't provide python3.
This commit changes test-runner.py to also use /usr/bin/python3,
enabling the change during buildtime and fixing a minor lintian issue
for those Debian packages, which depend on a specific python version
(python3/python2).

Signed-off-by: Stoiko Ivanov <[email protected]>
files in dist_*_SCRIPTS get installed with 0755, those in dist_*_DATA
with 0644. This commit moves all .kshlib, .shlib and .cfg files in the
testsuite to dist_pkgdata_DATA, and removes the shebang from
zpool_import.kshlib.

This ensures that the files are installed with appropriate permissions
and silences some warnings from lintian

Signed-off-by: Stoiko Ivanov <[email protected]>
@siv0 siv0 force-pushed the minor_lintian_fixes branch from 78c28f4 to e2390b7 Compare May 27, 2019 11:35
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels May 28, 2019
behlendorf pushed a commit that referenced this pull request May 28, 2019
files in dist_*_SCRIPTS get installed with 0755, those in dist_*_DATA
with 0644. This commit moves all .kshlib, .shlib and .cfg files in the
testsuite to dist_pkgdata_DATA, and removes the shebang from
zpool_import.kshlib.

This ensures that the files are installed with appropriate permissions
and silences some warnings from lintian

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: loli10K <[email protected]>
Reviewed by: John Kennedy <[email protected]>
Signed-off-by: Stoiko Ivanov <[email protected]>
Closes #8803
behlendorf pushed a commit that referenced this pull request Jun 7, 2019
In commit 6e72a5b python scripts which
work with python2 and python3 changed the shebang from /usr/bin/python
to /usr/bin/python3. This gets adapted by the build-system on systems
which don't provide python3.
This commit changes test-runner.py to also use /usr/bin/python3,
enabling the change during buildtime and fixing a minor lintian issue
for those Debian packages, which depend on a specific python version
(python3/python2).

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: loli10K <[email protected]>
Reviewed by: John Kennedy <[email protected]>
Signed-off-by: Stoiko Ivanov <[email protected]>
Closes #8803
behlendorf pushed a commit that referenced this pull request Jun 7, 2019
files in dist_*_SCRIPTS get installed with 0755, those in dist_*_DATA
with 0644. This commit moves all .kshlib, .shlib and .cfg files in the
testsuite to dist_pkgdata_DATA, and removes the shebang from
zpool_import.kshlib.

This ensures that the files are installed with appropriate permissions
and silences some warnings from lintian

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: loli10K <[email protected]>
Reviewed by: John Kennedy <[email protected]>
Signed-off-by: Stoiko Ivanov <[email protected]>
Closes #8803
allanjude pushed a commit to allanjude/zfs that referenced this pull request Jun 7, 2019
files in dist_*_SCRIPTS get installed with 0755, those in dist_*_DATA
with 0644. This commit moves all .kshlib, .shlib and .cfg files in the
testsuite to dist_pkgdata_DATA, and removes the shebang from
zpool_import.kshlib.

This ensures that the files are installed with appropriate permissions
and silences some warnings from lintian

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: loli10K <[email protected]>
Reviewed by: John Kennedy <[email protected]>
Signed-off-by: Stoiko Ivanov <[email protected]>
Closes openzfs#8803
allanjude pushed a commit to allanjude/zfs that referenced this pull request Jun 15, 2019
files in dist_*_SCRIPTS get installed with 0755, those in dist_*_DATA
with 0644. This commit moves all .kshlib, .shlib and .cfg files in the
testsuite to dist_pkgdata_DATA, and removes the shebang from
zpool_import.kshlib.

This ensures that the files are installed with appropriate permissions
and silences some warnings from lintian

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: loli10K <[email protected]>
Reviewed by: John Kennedy <[email protected]>
Signed-off-by: Stoiko Ivanov <[email protected]>
Closes openzfs#8803
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.

4 participants