'git ubuntu build-source -v --sign --for-merge' uses Debian instead of Ubuntu containers

Bug #1727590 reported by Steve Langasek on 2017-10-26
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
usd-importer
High
Nish Aravamudan

Bug Description

The documentation at <https://wiki.ubuntu.com/UbuntuDevelopment/Merging/GitWorkflow#Git_workflow_for_merging> says to use the command 'git ubuntu build-source -v --sign --for-merge' to build a source package for testing. I have run this command, and it tells me that it's trying to launch Debian sid in a lxd container.

I haven't looked to see why it wants to do this (the actual container launch failed because I happen to currently be running an upstream kernel for debugging, so zfs support is missing; so whatever it would tell me next, it never gets that far). But there shouldn't be any reason to download and run a Debian container as part of an Ubuntu source package build. If this is for retrieving the orig tarballs, why isn't this already in the git repo via pristine-tar as part of the import? And if there is some reason it shouldn't be done as part of the import, why would we need a Debian container image when debian-keyring + apt is all that's needed to securely download the orig tarballs into a local target?

Related branches

Robie Basak (racb) wrote :

Technically I believe that to build a source package, you need to install build dependencies and run the clean target first (which may depend on build dependencies). This may for example produce a control from a control.in, or similar, via a build dependency. That's why we're doing this in a container by default.

In the common case you can get away without this, but I wanted to avoid having to give first time contributors excuses about why the default case doesn't work. So I'm going for a container-based build being the default so that all source packages in the archive (including historical ones where possible) can be built without exception as much as possible.

I expect experienced developers will want to use --no-lxd by default. I intend for there to be some kind of per-user configuration option to make this this automatic for them.

This is currently by design, so this is Invalid I think. But I appreciate your option. Feel free to tell me why I'm wrong right here :)

Changed in usd-importer:
status: New → Invalid
Robie Basak (racb) wrote :

Oh. Or are you telling me it's wrong to use a *Debian* container as opposed to an Ubuntu one, rather than any container at all? In that case that sounds valid.

Changed in usd-importer:
status: Invalid → Incomplete
Robie Basak (racb) on 2017-10-26
tags: added: merge

On Thu, Oct 26, 2017 at 11:53:48AM -0000, Robie Basak wrote:
> Oh. Or are you telling me it's wrong to use a *Debian* container as
> opposed to an Ubuntu one, rather than any container at all? In that case
> that sounds valid.

Yes, this.

(I have also opined that the default should be to not run the clean target
for building the source package, but I understand why you disagree)

Changed in usd-importer:
status: Incomplete → New
Robie Basak (racb) on 2017-10-26
summary: - 'git ubuntu build-source -v --sign --for-merge' should not require
- Debian containers
+ 'git ubuntu build-source -v --sign --for-merge' uses Debian instead of
+ Ubuntu containers
Changed in usd-importer:
status: New → Triaged
importance: Undecided → High
milestone: none → 1.0
Nish Aravamudan (nacc) wrote :

Hello Steve,

On Thu, Oct 26, 2017 at 7:24 AM, Steve Langasek
<email address hidden> wrote:
> On Thu, Oct 26, 2017 at 11:53:48AM -0000, Robie Basak wrote:
>> Oh. Or are you telling me it's wrong to use a *Debian* container as
>> opposed to an Ubuntu one, rather than any container at all? In that case
>> that sounds valid.
>
> Yes, this.
>
> (I have also opined that the default should be to not run the clean target
> for building the source package, but I understand why you disagree)

At what point did you run `git ubuntu build-source`? Was it after `git
ubuntu merge finish` or after the rebase onto new/debian?

What snap risk was this? stable or edge? Or was this using the
edge/test-fixes branch?

Thanks,
Nish

Steve Langasek (vorlon) wrote :

On Thu, Oct 26, 2017 at 03:01:39PM -0000, Nish Aravamudan wrote:
> On Thu, Oct 26, 2017 at 7:24 AM, Steve Langasek
> <email address hidden> wrote:
> > On Thu, Oct 26, 2017 at 11:53:48AM -0000, Robie Basak wrote:
> >> Oh. Or are you telling me it's wrong to use a *Debian* container as
> >> opposed to an Ubuntu one, rather than any container at all? In that case
> >> that sounds valid.

> > (I have also opined that the default should be to not run the clean target
> > for building the source package, but I understand why you disagree)

> At what point did you run `git ubuntu build-source`? Was it after `git
> ubuntu merge finish` or after the rebase onto new/debian?

Following the documentation at
<https://wiki.ubuntu.com/UbuntuDevelopment/Merging/GitWorkflow#Git_workflow_for_merging>,
I ran it after the 'git rebase new/debian' step.

> What snap risk was this? stable or edge? Or was this using the
> edge/test-fixes branch?

Stable.

Nish Aravamudan (nacc) wrote :

On Thu, Oct 26, 2017 at 10:54 AM, Steve Langasek
<email address hidden> wrote:
> On Thu, Oct 26, 2017 at 03:01:39PM -0000, Nish Aravamudan wrote:
>> On Thu, Oct 26, 2017 at 7:24 AM, Steve Langasek
>> <email address hidden> wrote:
>> > On Thu, Oct 26, 2017 at 11:53:48AM -0000, Robie Basak wrote:
>> >> Oh. Or are you telling me it's wrong to use a *Debian* container as
>> >> opposed to an Ubuntu one, rather than any container at all? In that case
>> >> that sounds valid.
>
>> > (I have also opined that the default should be to not run the clean target
>> > for building the source package, but I understand why you disagree)
>
>> At what point did you run `git ubuntu build-source`? Was it after `git
>> ubuntu merge finish` or after the rebase onto new/debian?
>
> Following the documentation at
> <https://wiki.ubuntu.com/UbuntuDevelopment/Merging/GitWorkflow#Git_workflow_for_merging>,
> I ran it after the 'git rebase new/debian' step.

Thanks for pointing this out, this is a bug in the newer build
implementation (LXD based) and the old documentation.

We use HEAD's debian/changelog to figure out what we're building for,
and since there is no Ubuntu debian/changelog entry at the top (there
would be if finish had been run), we assume it's a Debian publication.
The edge snap would have the same behavior, but the test-fixes branch
should have a build flag --lxd-image that can be used to override the
auto-detection behavior. I think it might make sense to have for-merge
also override the behavior.

Nish Aravamudan (nacc) on 2018-01-08
Changed in usd-importer:
assignee: nobody → Nish Aravamudan (nacc)
milestone: lp-beta → future
Robie Basak (racb) wrote :

> We use HEAD's debian/changelog to figure out what we're building for...

I wonder if we should revisit this, or have code changes already done this by default? Perhaps we should always build for Ubuntu unless overridden. Considering Scott's MP in https://code.launchpad.net/~smoser/usd-importer/+git/usd-importer/+merge/358198, it looks like we're already defaulting to Ubuntu for "unstable". Then we'll only build on Debian for Debian's stable branches. Is that too surprising given that it's an edge case?

Scott Moser (smoser) wrote :

I suspect that Steve found this either
a.) before the change landed making 'unstable' build on ubuntu
b.) by trying to build something with 'experimental'.

As I said in the commit messages, this is "git ubuntu". So personally I think its reasonable to build in ubuntu unless overridden.

Steve Langasek (vorlon) wrote :

> Perhaps we should always build for Ubuntu unless overridden.

I agree with this.

> Then we'll only build on Debian for Debian's stable branches.
> Is that too surprising given that it's an edge case?

The number of times I merge from a Debian stable release update is exactly 0, so if someone wants this I don't object. But if nobody's specifically asked for this to build on Debian by default then I would suggest that it should also default to Ubuntu.

Scott Moser (smoser) wrote :

Updated the linked merge proposal.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers