[buildd-tools-devel] sbuild just hangs

[Roger Leigh]

On 01/10/2015 14:23, Johannes Schauer wrote:
> Quoting Roger Leigh (2015-08-01 10:17:51)
>> On 31/07/2015 05:58, Johannes Schauer wrote:
>>> Quoting Roger Leigh (2015-07-31 01:06:56)
>>>> On 30/07/2015 20:10, Johannes Schauer wrote:

>>>>> Secondly, whenever I saw somebody use the -d argument, they would just
>>>>> use it as a way to select the chroot to build in. But in addition to that
>>>>> it will also override the value set in debian/changelog, thus possibly
>>>>> leading to wrong uploads as the argument to -d is put in the resulting
>>>>> .changes file. The sbuild man page doesn't even mention this effect of
>>>>> the -d option which is probably why it's used so much. The help text in
>>>>> the configuration file explains better what $distribution does but the
>>>>> sbuild wiki even goes so far to suggest putting $distribution='sid' in
>>>>> the configuration file which means that unless the user is really aware
>>>>> of the implications of -d, they will always create .changes file with sid
>>>>> as the distribution no matter what they put in debian/changelog.  Is this
>>>>> "feature" of putting a different value in the resulting .changes file
>>>>> than the maintainer put in their debian/changelog useful for anything?
>>>>
>>>> Yes.  In the general case for autobuilding, we don't have the
>>>> distribution until after the build starts--it comes from the changelog
>>>> not the dsc, so we don't have the information until after we're already
>>>> running in the chroot.  This could be rearchitected, but it would be
>>>> quite painful and come with a significant cost since you'd need to copy
>>>> the downloaded source into the build chroot; and when all the
>>>> information you have is foo_version you can't even download it without
>>>> the apt package list from inside the chroot, so you have to make an
>>>> up-front decision about the chroot to use before you even get the
>>>> source.  This even applies to local sources--if you build from a .dsc,
>>>> you have to unpack it to get this information, and that could be very
>>>> expensive for large source packages.
>>>
>>> Okay, I understand that this would be hard to do when using sbuild so that it
>>> `apt-get source` the source package to build itself but I'd argue that this
>>> really only is done by autobuilders which will only have one proper
>>> configuration they will never change anyways.
>>>
>>> As a normal developer, you are interested in building the source you already
>>> have downloaded, either in its still packed or already unpacked state. But even
>>> in its packed state, is it not easy to acquire the distribution in the very
>>> beginning, just by doing something along the lines of:
>>>
>>>        tar --to-stdout -xf package_version.debian.tar.xz debian/changelog
>>>
>>> And at that point you could then state in the documentation that the only
>>> situation in which you might want to use the -d option is if sbuild is
>>> instructed to retrieve the source itself.
>>
>> This could certainly work, though it's not a complete solution--what
>> about .diff.gz?  Are there additional variants we need to cope with?  Is
>> it liable to break with future packaging format changes?
>>
>> The point I want to make here is that while a quick hack to extract it
>> is *possible*, it needs to be *absolutely robust* in every case.  And
>> since that likely requires unpacking with dpkg-source it would become
>> very expensive.  Please don't add hacky circumventions of the standard
>> tools for processing the package formats; this will inevitably lead to
>> problems, and would be bad practice.  Some of the improvements I made to
>> sbuild were to remove such problematic hacks and use the standard
>> tools--they always come back to bite.
>
> I agree. I think the best place to implement the feature to retrieve packaging
> information (like debian/changelog) from a source package of any format would
> be in dpkg. Thus I filed a bug with a patch for dpkg here: #797879.
>
> With that option, sbuild can just call:
>
>     dpkg-source --debian-tarfile mypkg.dsc \
>        | tar --extract --to-stdout debian/changelog \
>        | dpkg-parsechangelog --show-field Distribution
>
>>> You also didn't answer my question from above: in which scenario would
>>> anybody using sbuild want that the distribution in the output .changes be
>>> different from the one in debian/changelog?
>>
>> I'm not sure of any good reasons off the top of my head, but it's likely
>> that there are reasons for it.
>>
>> Just a word of caution: sbuild is old, and has a long history.  I have
>> attempted to clean it up somewhat over the years, and have removed a lot
>> of obsolete cruft.  But it's often hard to tell for a given feature
>> whether it's actually useful or if it's obsolete without looking back in
>> the history.  But there are a lot of people using sbuild for all sorts
>> of varied purposes, and more often than not people *do* use many of the
>> more esoteric features and will shout if they are broken or removed, so
>> I would advise against removing things just because the reason for their
>> use isn't immediately obvious; definitely check thoroughly first and
>> then think again before deleting it.  Likewise, please be very cautious
>> about changing the behaviour or options in use--there are a lot of
>> people using sbuild who would find their workflows break.
>
> Right, I'll abstain from changing the behaviour of existing options.

Just to clarify, I wouldn't want to put anyone off making any changes at 
all to existing options.  I just want to emphasise that such changes 
need to be made with caution.

>>>>> Thirdly, I think that the default sbuild configuration should do the
>>>>> right thing when building Debian packages. This means, that even if the
>>>>> target release in debian/changelog is UNRELEASED, then sbuild should
>>>>> default to build for "sid" (or another default value settable in the
>>>>> sbuildrc). If the distribution in debian/changelog is sid, then sbuild
>>>>> should also consider the chroot called "unstable" and the other way
>>>>> round.
>>>>
>>>> I once implemented exactly this functionality, and I was castigated for
>>>> it when people uploaded packages unintentionally built against the wrong
>>>> distribution, so it was backed out rather rapidly.  The intention was
>>>> good, and it even warns you about it in colour that there's a mismatch,
>>>> but it's inevitable that people will fail to see it and also fail to
>>>> check the .changes.
>>>
>>> Again, the problem seems not about chroot selection but about sbuilds behaviour
>>> to change the distribution in the output .changes file. Why would sbuild not
>>> just keep UNRELEASED in that field if that was the value in debian/changelog?
>>
>> It probably comes down to the behaviour of always manually specifying
>> the distribution.  You would need to investigate the historical reasons
>> for doing this before changing anything.
>
> Even after investigation I am unable to find a reason why somebody building a
> package which says UNRELEASED in their debian/changelog would like to have
> anything else being put automatically into the generated .changes file.

I think this sounds sensible, and certainly wouldn't object to that 
being done.  I think the distribution on debian/changelog should be what 
goes into the .changes.  I still can't think of any obvious cases where 
this wouldn't be true, e.g. backports use proper changelog entries. 
NMUs use additional changelog entries.  I think this would be save to fix.

>>>> I think you could make a case for UNRELEASED defaulting to unstable.  For
>>>> everything else it's risky ground.  I don't think using sid is generally
>>>> advised?  Better to use unstable everywhere?
>>>
>>> I don't know about any resource that claims that one is preferred over the
>>> other.
>>
>> dch has always used unstable AFAICT, if that's any guide.  I've never
>> seen anyone using "sid" in the last 18 years, so I would suggest that
>> this is not typical usage by any means.
>
> The Debian wiki page for sbuild used to say "sid" everywhere. I changed this
> now to "unstable" because:
>
>   - dch does it that way
>   - dak seems to map sid to unstable, using unstable in the first place prevents
>     that mapping
>   - the sbuild test suite uses unstable
>
> I guess the easiest fix to map UNRELEASED to a sid/unstable chroot would be to
> have UNRELEASED by an alias of the sid/unstable schroot name. This could be
> achieved by letting the user pass their custom schroot aliases as an argument
> to sbuild-createchroot. I just reported wishlist bug #800593 so that I do not
> loose track of this.

Thanks, this also sounds sensible.

>>>>> Fourthly, the -c option choose the chroot to use. But I think one of the
>>>>> reasons why people rather use -d to choose the chroot is because it's so
>>>>> much shorter to write -d sid compared to writing -c sid-amd64-sbuild.
>>>>> There should be an equally short option to select the chroot but
>>>>> *without* overwriting the distribution value in the resulting .changes
>>>>> file.
>>>> This is down to configuration.  If you've configured schroot with the
>>>> appropriate name(s) and aliases then you never, ever, need to use
>>>> --chroot since the -d name will always map to a valid chroot.  Even for
>>>> cross-building.
>
> Right, but we don't want to use --dist because of its effect on the created
> .changes file. And we do not want to changed what --dist does because of
> backward compatibility.

I think this one might be where backward compatibility could be 
sacrificed in favour of general correctness and understandable default 
behaviour.  If you upload to a different distribution than what's in the 
changelog, that's IMO a bit broken, and not something sbuild needs to 
support.

> Bug #800593 would also solve this. Then, given the user running
> sbuild-createchroot specifies the right aliases, using -c would be painless as
> the name could be arbitrarily short.

>>>>>> Note that you can add "aliases=unstable-amd64-sbuild" or equivalent for
>>>>>> your distribution/arch to your schroot configuration, which will make it
>>>>>> select the appropriate distribution when you use "-d unstable".
>>>>>
>>>>> Fifthly, I think this should be the default behaviour for the sid/unstable
>>>>> pair.  I'm just unsure whether schroot should set this default or whether
>>>>> sbuild should translate sid to unstable and the other way round?
>>>>
>>>> It's not the job of schroot to impose any policy here.  It knows nothing
>>>> about sbuild.  On the sbuild side, sbuild-createchroot is the place
>>>> where such mappings could be added as aliases at initial debootstrap of
>>>> the chroot.  But as a general point, I would question the use of "sid"
>>>> in place of "unstable".  Just add the "unstable", "unstable-amd64" or
>>>> "unstable-amd64-sbuild" alias(es) and it will automatically select the
>>>> right chroot.  However, automatically adding the unstable-amd64-sbuild
>>>> alias if you debootstrap a sid chroot is absolutely fine--though it
>>>> might need a check for the name already being in use.
>>>
>>> Okay, but is sbuild not supposed to potentially also work with other backends
>>> than schroot in the future? Would you also say that if other backends are added
>>> later then it should be them to do the mapping between unstable/sid?
>>
>> The backends just provide access to the virtualised build environments.
>>    In the case of the sudo backend, it's just a directory of symlinks.
>> They are all simple--it's not their place to provide or impose any sort
>> of policy.  You specify a distribution to build for, and you get that.
>> No more or less.
>>
>> I think this is a bit of a problem in search of a solution.  It could be
>> made slightly nicer by automatically adding additional aliases with
>> sbuild-createchroot so you can use sid or unstable, and for stable and
>> testing we could add -proposed-updates as well, etc.  But for codenames,
>> it's not realistic to maintain a mapping of them in the tool, so the
>> logical names should be preferred.  Note that sbuild is used in
>> downstream distributions and by people outside Debian doing custom
>> repos, so it needs to be kept generic.  Historically it hardcoded all this
>> and I ripped it out due to the breakage and inflexibility it caused.
>
> Right, indeed hardcoding this would be the wrong way and unmaintainable. But I
> think being able to supply user specified aliases during the running of
> sbuild-createchroot fixes this nicely as well.
>
> Lets summarize the problem before I present the solution:
>
> The --dist option currently has to be specified in many situations like
> building a .dsc directly or letting sbuild use `apt-get source` to retrieve the
> source package within the chroot. Here, the --dist option is used to pick the
> right chroot because the Distribution value of debian/changelog can only be
> accessed after the .dsc is unpacked or after the source package has been
> retrieved by apt inside the chroot. The problem with the --dist option is that
> it may change the value of the Distribution field of the resulting .changes
> file to a value different from the value in debian/changelog, leading to
> potential wrong uploads (though I heard that dput-ng checks for this and
> prevents this kind of uploads).
>
> We also don't want to change this behaviour of the --dist option because we
> fear that there exist real world use cases for what it does which we would then
> break.
>
> As a solution: what about using the -c option more?
>
> Currently this is not possible, but technically (with the right schroot
> aliases) it should be possible to do:
>
>   $ sbuild -c sid mypkg_1.0.dsc    # building a local .ds
>   $ sbuild -c sid mypkg_1.0        # using apt-get to retrieve
>
> Because in both cases, we initially only need to know the chroot into which the
> source package should be unpacked or where apt should retrieve the source
> package. Once this is done, sbuild can investigate the Distribution value in
> debian/changelog and put that into the resulting .changes file.
>
> If the user still wants to override the value in the .changes file, then they
> are free to use the --dist option.
>
> In the future, once bug #797879 is fixed, this can become even easier, because
> then the -c option can be dropped when working on a .dsc and is only needed
> when letting apt-get inside the chroot do the downloading.
>
> Or, if the user specified the $chroot option in their sbuildrc, then they will
> never again have to use the --dist option. Except when they want to build in a
> different chroot than the default one - which seems like a think that should be
> completely expected (in contrast to the side effects of the --dist option which
> are quite unexpected).
>
> Does this sound like a solution that would make the --dist option useless in
> most use cases and can thus avoid the undesirable side effects of that option?

I think this would be a reasonable way of doing things while also 
preserving backward compatibility.  On the other hand, I think it would 
be entirely reasonable to always use the changelog distribution in the 
.changes, the exception being when --dist is used to make an NMU, in 
which case the provided distribution will go into both the changelog 
entry and the changes.  That would make --dist and --chroot 
interchangeable; the exception being NMUs where they would have 
different roles; --dist providing the distribution name, and --chroot 
specifying the chroot for building.


Regards,
Roger