Bug#934721: sbuild: Lintian run via sbuild produces different output than running lintian manually with the same options
Hi Josch,
Here's an updated patch.
Now .changes.new is written to $build_dir, then renamed to .changes, and
copied from the chroot to sys_build_dir.
I removed the call to unlink() since we want to keep the .changes file
in $build_dir so it can be used by lintian.
Also, I saw in HACKING that the emacs perl-mode style is used. I don't
use emacs, and unfortunately a web search didn't turn up the specifics
of what that styling entails. So I tried to match the style the best I
could. Apologies if it's off slightly.
Let me know if you see any issues with the patch.
Thanks,
Bill
diff --git a/lib/Sbuild/Build.pm b/lib/Sbuild/Build.pm
index f5660148..5e60436f 100644
--- a/lib/Sbuild/Build.pm
+++ b/lib/Sbuild/Build.pm
@@ -2601,8 +2601,9 @@ sub build {
}
my $sys_build_dir = $self->get_conf('BUILD_DIR');
- if (!open( F2, ">$sys_build_dir/$changes.new" )) {
- $self->log("Cannot create $sys_build_dir/$changes.new: $!\n");
+ my $F2 = $session->get_write_file_handle("$build_dir/$changes.new");
+ if (!$F2) {
+ $self->log("Cannot create $build_dir/$changes.new\n");
$self->log("Distribution field may be wrong!!!\n");
if ($build_dir) {
if(!$session->copy_from_chroot("$build_dir/$changes", ".")) {
@@ -2611,14 +2612,21 @@ sub build {
}
} else {
$pchanges->output(\*STDOUT);
- $pchanges->output(\*F2);
-
- close( F2 );
- rename("$sys_build_dir/$changes.new", "$sys_build_dir/$changes")
- or $self->log("$sys_build_dir/$changes.new could not be " .
- "renamed to $sys_build_dir/$changes: $!\n");
- unlink("$build_dir/$changes")
- if $build_dir;
+ $pchanges->output(\*$F2);
+
+ close( $F2 );
+
+ $session->rename("$build_dir/$changes.new", "$build_dir/$changes");
+ if ($? == 0) {
+ $self->log("$build_dir/$changes.new could not be " .
+ "renamed to $build_dir/$changes: $!\n");
+ $self->log("Distribution field may be wrong!!!");
+ }
+ if ($build_dir) {
+ if (!$session->copy_from_chroot("$build_dir/$changes", "$sys_build_dir")) {
+ $self->log("Could not copy $build_dir/$changes to $sys_build_dir");
+ }
+ }
}
return $pchanges;
Bug#934721: sbuild: Lintian run via sbuild produces different output than running lintian manually with the same options
On Thu, Aug 15, 2019 at 01:38:48PM +0200, Johannes Schauer wrote:
>
> > Also, after the new .changes file is written, there's an attempt to delete
> > the old one in , but it seems to fail silently. So I assume this
> > is a bug, rather than an intentional choice. (Also, the variables in
> > question
> > are very similarly named, so I think it would be an easy mistake to make).
>
> I assume you are referring to this unlink call?
>
> https://sources.debian.org/src/sbuild/0.78.1-2/lib/Sbuild/Build.pm/#L2620
Yes.
>
> > I've attached a small patch that makes it use the modified .changes file
> > instead of the unmodified one. On my system, this makes it behave as I
> > would
> > expect. That is, lintian run via sbuild behaves the same way as lintian run
> > manually, since they're now using the exact same .changes file.
>
> that your patch works for you is coincidental. By using
> $self->get_conf('BUILD_DIR') instead of $self->get('Build Dir') you will end
> up
> using the output directory of the build artifacts *outside* the chroot. See
> the
> documentation for the BUILD_DIR config option in sbuild.conf(5). That it works
> for you might mean that you have set up your chroots in a way such that
> lintian
> inside the chroot has access to the path outside of it. Maybe you are building
> in /tmp and have that mounted inside as well, for example?
You're right. I forgot that for troubleshooting purposes I modified my schroot
to mount my homedir. Good catch.
>
> If you would like to prepare a patch that fixes this issue, then what should
> actually happen is probably that the "copy_changes()" subroutine modifies the
> .changes files inplace inside the chroot so that when Lintian is called later,
> it sees the same files as the ones that get copied to the outside by
> "copy_changes()".
Sure. I'll see what I can do.
Thanks.
Bill
Bug#934721: sbuild: Lintian run via sbuild produces different output than running lintian manually with the same options
Hi Bill,
Quoting Bill Blough (2019-08-15 03:43:51)
> I've had a chance to do some more exploration.
>
> Lintian is indeed getting run with a different .changes file than what
> is output to screen/disk.
>
> The package build creates a changes file in the temporary
> that contains a Distribution of UNRELEASED. This happens even if the
> distribution is specified with -d.
>
> The .changes file later gets written to BUILD_DIR, with the Distribution
> field set to what was specified by -d.
>
> However, lintian is run against the original (Dist: UNRELEASED) .changes
> file left in , not the modified version written in BUILD_DIR.
>
> It seems to me that lintian should be run against the modified .changes
> file that is provided to the user after the build, rather than the
> leftover one in that is different.
I agree. The lintian checks done by sbuild should be carried out on the same
artifacts as they are created by sbuild outside of the chroot.
> Also, after the new .changes file is written, there's an attempt to delete
> the old one in , but it seems to fail silently. So I assume this
> is a bug, rather than an intentional choice. (Also, the variables in question
> are very similarly named, so I think it would be an easy mistake to make).
I assume you are referring to this unlink call?
https://sources.debian.org/src/sbuild/0.78.1-2/lib/Sbuild/Build.pm/#L2620
> I've attached a small patch that makes it use the modified .changes file
> instead of the unmodified one. On my system, this makes it behave as I would
> expect. That is, lintian run via sbuild behaves the same way as lintian run
> manually, since they're now using the exact same .changes file.
that your patch works for you is coincidental. By using
$self->get_conf('BUILD_DIR') instead of $self->get('Build Dir') you will end up
using the output directory of the build artifacts *outside* the chroot. See the
documentation for the BUILD_DIR config option in sbuild.conf(5). That it works
for you might mean that you have set up your chroots in a way such that lintian
inside the chroot has access to the path outside of it. Maybe you are building
in /tmp and have that mounted inside as well, for example?
If you would like to prepare a patch that fixes this issue, then what should
actually happen is probably that the "copy_changes()" subroutine modifies the
.changes files inplace inside the chroot so that when Lintian is called later,
it sees the same files as the ones that get copied to the outside by
"copy_changes()".
Thanks!
cheers, josch
signature.asc
Description: signature
Bug#934721: sbuild: Lintian run via sbuild produces different output than running lintian manually with the same options
I've had a chance to do some more exploration.
Lintian is indeed getting run with a different .changes file than what
is output to screen/disk.
The package build creates a changes file in the temporary
that contains a Distribution of UNRELEASED. This happens even if the
distribution is specified with -d.
The .changes file later gets written to BUILD_DIR, with the Distribution
field set to what was specified by -d.
However, lintian is run against the original (Dist: UNRELEASED) .changes
file left in , not the modified version written in BUILD_DIR.
It seems to me that lintian should be run against the modified .changes
file that is provided to the user after the build, rather than the
leftover one in that is different.
Also, after the new .changes file is written, there's an attempt to delete the
old one in , but it seems to fail silently. So I assume this
is a bug, rather than an intentional choice. (Also, the variables in
question are very similarly named, so I think it would be an easy
mistake to make).
I've attached a small patch that makes it use the modified .changes file
instead of the unmodified one. On my system, this makes it behave as I
would expect. That is, lintian run via sbuild behaves the same way as
lintian run manually, since they're now using the exact same .changes file.
Best regards,
Bill
diff --git a/lib/Sbuild/Build.pm b/lib/Sbuild/Build.pm
index f5660148..83dc295f 100644
--- a/lib/Sbuild/Build.pm
+++ b/lib/Sbuild/Build.pm
@@ -1684,7 +1684,7 @@ sub run_lintian {
my $pipe = $session->pipe_command(
{ COMMAND => \@lintian_command,
PRIORITY => 0,
- DIR => $self->get('Build Dir'),
+ DIR => $self->get_conf('BUILD_DIR'),
PIPE => "in"
});
if (!$pipe) {
Bug#934721: sbuild: Lintian run via sbuild produces different output than running lintian manually with the same options
Hi Josch, Thanks for taking the time to give that explanation. In my testing, I observed exactly what you said - using '-d' sets the Distribution field in the .changes file. Not using '-d' causes the Distribution to be populated with the distribution value from the d/changelog entry But I feel like there's still an issue, or I'm still not understanding something correctly. When sbuild calls lintian, it passes the filename of the .changes file. If my understanding is correct, using or not using '-d' has already affected the contents of the .changes file at that point. Then linitan runs against that .changes file, and gets all of its information from there. So shouldn't running lintian outside of sbuilt using the *exact same* .changes file produce the same output? I would think yes, unless- 1) the .changes file passed to lintian is different than the .changes file written to the log and left on disk after the build or 2) the output from lintian is somehow altered/modified by sbuild. Neither of those seem to be the case, but I'm not 100% sure. However, either of those could explain the discrepancy. > > Do you see any way sbuild could improve to be less confusing? I think most of the docs are pretty clear. But I guess that depends on if there's really an issue as I suspect, or if I'm just greatly misunderstanding thing. I guess I'll let you know once we figure out which one :-) Best regards, Bill
Bug#934721: sbuild: Lintian run via sbuild produces different output than running lintian manually with the same options
Hi William, Quoting William Blough (2019-08-14 02:52:00) > To reproduce- > > 1. For an arbitrary package (I used hello in my example), create a new > debian/changelog entry with the distribution set to UNRELEASED. > > 2. build with sbuild, specifying a target distribution, and enable lintian: > sbuild -d unstable --run-lintian --lintian-opts=-v > > 3. Note from the output/log that lintian *does not* emit the > "unreleased-changes" tag. > > 4. Find the .changes file produced by the sbuild build > > 5. Run lintian against the changes file from 4, with the same options > used for linitan in 2: > lintian hello_2.10-2.1_amd64.changes -v > > 6. Note that lintian *does* emit the "unreleased-changes" tag for the > changes file produced by the earlier build with sbuild. > > > Expected output: > "unreleased-changes" tag emitted by lintian when run under sbuild and when > run manually > > Observed output: > "unreleased-changes" tag only emitted by linitan when run manually. Tag > not emitted when lintian run by sbuild > > > I've attached a build log (with debug) for an example build, and a terminal > session log for the related manual lintian run. > > I've verified that this happens on both unstable and buster (inside > chroots), with their respective versions of sbuild and lintian. I > haven't tried earlier suites or versions. > > > I'm not 100% certain that this is an sbuild issue, but I've done > everything I can think of to rule out my configuration. And since > lintian seems to produce the correct behavior outside of sbuild, I > thought this would be the logical place to start the bug report. > > I'd appreciate any insight you can give, and would be happy to help > test/troubleshoot further. thanks a lot for your excellent bug report! Unfortunately, the effect you are seeing is not a bug but a feature back from the time before I started maintaining sbuild. To understand the situation, it is important to know what the -d option that you are using above actually does. >From the man page: -d, --dist=distribution Explicitly set the distribution for the package build. This will be selecting the correct chroot to use and also sets the value of the Distribution field in the created .changes file. Setting this option is necessary when giving sbuild a .dsc file or a plain source package name to build. In the latter case it speci‐ fies the distribution the source package is fetched from. This command line option sets the DISTRIBUTION configuration vari‐ able. See sbuild.conf(5) for more information. As you can see, this will set the Distribution field and thus the value will not be UNRELEASED anymore and thus lintian does not complain anymore. If you look closely at the output of sbuild, you will see that the "Distribution" value is highlighted in yellow. This is to indicate that the value was changed. Usually you should *not* use the -d parameter but instead set up your schroot config such that you don't need the -d parameter. This can be automated by specifying --alias=UNRELEASED when running sbuild-createchroot. What that option effectively does is to set aliases=UNRELEASED in the corresponding config file in /etc/schroot/chroot.d/. Do you see any way sbuild could improve to be less confusing? Thanks! cheers, josch signature.asc Description: signature
