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

Lintian integration tests #648

Merged
merged 4 commits into from Mar 26, 2014

Conversation

Projects
None yet
2 participants
@samcrang
Contributor

samcrang commented Feb 23, 2014

I was having some issues with lintian and packages generated by fpm. Specifically after this commit.

I think I've fixed this issue as well as adding some integration tests (hopefully might be useful to expand on these tests to make sure fpm is doing sane things).

I've also made some somewhat opinionated changes that I hope you're okay with:

  • Changed the box_urls in the Vagrantfile to use boxes that exist publicly (built and maintained by Puppet Labs).
  • Removed the check for /EMPTY in the Puppet manifest—this really shouldn't be required and indicates something wrong with the box.

Hope you like the changes! :-)

@samcrang

This comment has been minimized.

Contributor

samcrang commented Feb 23, 2014

Just realised the tests only pass for Wheezy, they fail on Squeeze with the following:

E: name: control-file-has-bad-owner conffiles 0/0 != root/root

0/0 and root/root should be equivalent so this seems like a bug in lintian (presumably it got fixed in the version that is in Wheezy). If we were to do things to fix this "problem" we'd end up breaking this unless we were to specifically branch on the host OS (which seems very ugly).

Probably makes sense to only test against the current stable? I'll remove the entry for Squeeze from the Vagrantfile.

allconfigs << p.gsub("#{staging_path}/", '') if File.file? p
if File.file? p
allconfigs << p.gsub("#{staging_path}/", '') if File.file? p
FileUtils.chmod 0755, p

This comment has been minimized.

@jordansissel

jordansissel Mar 7, 2014

Owner

Why would a config file need to be chmod 755? What lintian rule complains about this? (I don't see it when I run lintian)

This comment has been minimized.

@samcrang

samcrang Mar 7, 2014

Contributor

I'll double check this over the weekend, but I did the whole work TDD so I'm pretty sure it was to fix an actual failing test. I'll get back to you.

This comment has been minimized.

@samcrang

samcrang Mar 8, 2014

Contributor

I've just had a look at this—without the chmod change I get the following error from lintian:

E: name: non-standard-file-permissions-for-etc-init.d-script etc/init.d/test 0644 != 0755

I'm using the Vagrant boxes that I replaced in the Vagrantfile because I wasn't able to download the ones that were there. The machine I'm using has the following version of lintian:

$ lintian --version
Lintian v2.5.10.4
$ lsb_release -a
No LSB modules are available.
Distributor ID: Debian
Description:    Debian GNU/Linux 7.3 (wheezy)
Release:    7.3
Codename:   wheezy

This comment has been minimized.

@samcrang

samcrang Mar 8, 2014

Contributor

Thinking about this it probably doesn't make sense for fpm to "fix" this problem—it should probably just use the permissions that the user tells it to—they may have legitimate reasons not to want 755.

@@ -611,7 +614,10 @@ def write_conffiles
File.open(control_path("conffiles"), "w") do |out|
# 'config_files' comes from FPM::Package and is usually set with
# FPM::Command's --config-files flag
allconfigs.each { |cf| out.puts(cf) }
allconfigs.each do |cf|
# we need to put the leading / back to stop lintian complaining

This comment has been minimized.

@jordansissel

jordansissel Mar 7, 2014

Owner

Can you include the lintian rule that complains here? I see it in my tests as "relative-conffile" - do you agree?

This comment has been minimized.

@samcrang

samcrang Mar 7, 2014

Contributor

Yup, that's correct. I'll update the comment to be a bit more explicit.

@jordansissel

This comment has been minimized.

Owner

jordansissel commented Mar 7, 2014

I wonder if it's worth keeping a list of lintian complaints that we explicitly don't care about, such as:

  • changelog-file-missing-in-native-package
  • non-etc-file-marked-as-conffile
  • control-file-has-bad-permissions
  • no-copyright-file
  • extended-description-is-empty
  • bad-package-name
  • maintainer-name-missing
  • maintainer-address-malformed
  • unknown-section
  • non-standard-dir-perm

Any others we should definitely ignore?

@samcrang

This comment has been minimized.

Contributor

samcrang commented Mar 7, 2014

This sounds like a sensible idea, although maybe they should just be added to the --suppress-tags argument to lintian in the tests as and when they become a problem?

Also, I think we probably do care about control-file-has-bad-permissions--I think fpm should be setting this correctly. The others look like they're the kind of things that it's up to the end user to fix/ignore at their discretion.

@samcrang samcrang closed this Mar 8, 2014

@samcrang samcrang deleted the samcrang:lintian_integration_tests branch Mar 8, 2014

@samcrang samcrang restored the samcrang:lintian_integration_tests branch Mar 8, 2014

@samcrang samcrang reopened this Mar 8, 2014

@samcrang

This comment has been minimized.

Contributor

samcrang commented Mar 8, 2014

So, I did a rebase with your latest changes as well as your suggestions.

Hope this looks better than before.

jordansissel added a commit that referenced this pull request Mar 26, 2014

@jordansissel jordansissel merged commit 488fc97 into jordansissel:master Mar 26, 2014

prof-milki pushed a commit to prof-milki/xpm that referenced this pull request Dec 18, 2014

prof-milki pushed a commit to prof-milki/xpm that referenced this pull request Dec 27, 2014

jordansissel added a commit that referenced this pull request Apr 24, 2015

jordansissel added a commit that referenced this pull request Jun 20, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment