[SRU] liblxc-dev was built with LXC_DEVEL=1 in Ubuntu 22.04 and later releases

Bug #2039873 reported by Aleksandr Mikhalitsyn
26
This bug affects 2 people
Affects Status Importance Assigned to Milestone
lxc (Ubuntu)
Fix Released
Undecided
Unassigned
Jammy
Fix Released
Undecided
Unassigned
Mantic
Fix Released
Undecided
Unassigned
Noble
Fix Released
Undecided
Unassigned

Bug Description

[ Impact ]

LXC 5.0.0 was built with LXC_DEVEL=1 set for Jammy. But for release build we should have LXC_DEVEL=0.

LXC_DEVEL is a variable that appears in the /usr/include/lxc/version.h and then can be (and actually it is) used by other projects to detect if liblxc-dev is a development build or stable.

Having LXC_DEVEL=1 makes problems for the users who want to build projects those are depend on liblxc
from source (for example, LXD, go-lxc: https://github.com/canonical/lxd/pull/12420).

Q: Why it was not a problem for so long?
A: Because LXC API was stable for a long time, but recently we have extended liblxc API (https://github.com/lxc/lxc/pull/4260) and dependant package go-lxc was updated too (https://github.com/lxc/go-lxc/pull/166).
This change was developed properly to be backward compatible with the old versions of liblxc. But, there is a problem. If LXC_DEVEL=1 then the macro check VERSION_AT_LEAST (https://github.com/lxc/go-lxc/blob/ccae595aa49e779f7ecc9250329967aa546acd31/lxc-binding.h#L7) is disabled. That's why we should *not* have LXC_DEVEL=1 for *any* release build of LXC.

[ Test Plan ]

Install liblxc-dev package and check /usr/include/lxc/version.h file
LXC_DEVEL should be 0

[ Where problems could occur ]

Theoretically, build of a software which depends on liblxc-dev may start to fail
if it assumes that LXC_DEVEL is 1.

[ Other Info ]

-

Revision history for this message
Simon Déziel (sdeziel) wrote :
Revision history for this message
Aleksandr Mikhalitsyn (mihalicyn) wrote :

We have discussed that in the #lxd-dev IRC with Simon but I decided to post it here for others.

It looks like we need to patch 3 places:
https://git.launchpad.net/ubuntu/+source/lxc/tree/configure.ac?h=applied/ubuntu/jammy#n3
https://git.launchpad.net/ubuntu/+source/lxc/tree/configure?h=applied/ubuntu/jammy#n3498
https://git.launchpad.net/ubuntu/+source/lxc/tree/src/lxc/version.h?h=applied/ubuntu/jammy#n6

For some reason, the sources which are used for Jammy LXC 5.0.0 use autoconf/automake build system, at the same time upstream https://github.com/lxc/lxc/tree/lxc-5.0.0 uses meson.

removing https://git.launchpad.net/ubuntu/+source/lxc/tree/debian/patches/0003-meson-Set-DEVEL-flag-post-release.patch won't help as we don't use meson.

Revision history for this message
Aleksandr Mikhalitsyn (mihalicyn) wrote (last edit ):

Simon wanted to make a debdiff for this issue because he has an experience with that.

This is debdiff from me but this is the 1st debdiff in my life. Most likely I did something wrong :-)

What I did:

1. set email and name

$ export <email address hidden>"
$ export DEBFULLNAME="Your Name"

2. pull source package

$ pull-lp-source liblxc-dev jammy

3. make required source modifications

4. edit debian/changelog file properly and not forget to add LP issue link

$ debchange

5. form local source changes into patch

$ dpkg-source --commit

6. create a source package from sources

$ debuild -S -d

7. build a binary package and test it

$ debuild
$ dpkg -i your_package.deb

8. create a debdiff:

$ debdiff lxc_5.0.0~git2209-g5a7b9ce67-0ubuntu1.dsc lxc_5.0.0~git2209-g5a7b9ce67-0ubuntu2.dsc > debdiff.diff

Huge thanks to Stéphane Graber for initial instructions about the procedure.

Revision history for this message
Aleksandr Mikhalitsyn (mihalicyn) wrote :
Revision history for this message
Aleksandr Mikhalitsyn (mihalicyn) wrote :

I have updated a debdiff and removed the boilerplate comment in `0002-Ubuntu-set-LXC_DEVEL-to-0.patch as suggested by Simon Déziel

Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote :

The attachment "set LXC_DEVEL to 0" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

tags: added: patch
Revision history for this message
Aleksandr Mikhalitsyn (mihalicyn) wrote :

I have just added SRU template

description: updated
description: updated
Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in lxc (Ubuntu):
status: New → Confirmed
Revision history for this message
Robie Basak (racb) wrote :

Thank you for working on this.

Has this been fixed in the development release, and if so, how?

It's not clear to me that making this change is the appropriate thing to do in an SRU. How is LXC_DEVEL used in practice? Have you analysed known reverse dependencies to understand the impact of making this change? What did you find?

The only impact to users that I can understand from your explanation is that VERSION_AT_LEAST is disabled, causing builds outside the archive that use that macro to fail. Everything else seems to make the assumption that the correct way to fix this is to change LXC_DEVEL from 1 to 0, but without explaining why this is the minimal change possible.

Is there any other actual real world impact?

Could you just patch to make VERSION_AT_LEAST work instead, for SRU purposes, to minimise regression risk?

Please note the entire paragraph from https://wiki.ubuntu.com/StableReleaseUpdates that includes the following text:

"...requirements for stable updates are not necessarily the same as those in the development release..."

Revision history for this message
Aleksandr Mikhalitsyn (mihalicyn) wrote :

Dear Robie,

thanks for paying attention to this bug!

>Has this been fixed in the development release, and if so, how?

LXC_DEVEL is 1 in the development release:
https://github.com/lxc/lxc/blob/main/meson.build#L36

But LXC_DEVEL is 0 in *any* stable tag:
https://github.com/lxc/lxc/blob/lxc-5.0.3/meson.build#L36

And this is correct.

>It's not clear to me that making this change is the appropriate thing to do in an SRU. How is LXC_DEVEL used in practice?

LXC_DEVEL is used to determine if the liblxc is a cutting-edge development snapshot of the LXC or not.
So, it should be 1 *only* for the main branch of lxc. But in all stable version it is 0.

> Have you analysed known reverse dependencies to understand the impact of making this change? What did you find?

I have analyzed well-known reverse dependency go-lxc. It's used by LXD to communicate with liblxc C API.

>The only impact to users that I can understand from your explanation is that VERSION_AT_LEAST is disabled, causing builds outside the archive that use that macro to fail. Everything else seems to make the assumption that the correct way to fix this is to change LXC_DEVEL from 1 to 0, but without explaining why this is the minimal change possible.

Speaking honestly, I have no idea about other good ways to fix this. And this change seems to be a "minimal" for me because it does not change LXC code (and should not) it's just a matter of having proper build configuration.

>Is there any other actual real world impact?

I don't think that changing LXC_DEVEL to 0 can break any properly written code. For example, Debian folks have it disabled:
https://git.launchpad.net/ubuntu/+source/lxc/tree/meson.build?h=applied/debian/bookworm#n36

>Could you just patch to make VERSION_AT_LEAST work instead, for SRU purposes, to minimise regression risk?

Of course, we can patch go-lxc (go-lxc also part of the LXC project). But this will be a hacky and incorrect way to fix things.

Revision history for this message
Robie Basak (racb) wrote : Re: [Bug 2039873] Re: liblxc-dev was built with LXC_DEVEL=1 in Ubuntu Jammy/Kinetic
Download full text (3.5 KiB)

On Mon, Oct 23, 2023 at 04:27:16PM -0000, Aleksandr Mikhalitsyn wrote:
> >Has this been fixed in the development release, and if so, how?
>
> LXC_DEVEL is 1 in the development release:
> https://github.com/lxc/lxc/blob/main/meson.build#L36
>
> But LXC_DEVEL is 0 in *any* stable tag:
> https://github.com/lxc/lxc/blob/lxc-5.0.3/meson.build#L36

I meant the *Ubuntu* development release, not an upstream development
release.

> >It's not clear to me that making this change is the appropriate thing
> to do in an SRU. How is LXC_DEVEL used in practice?
>
> LXC_DEVEL is used to determine if the liblxc is a cutting-edge development snapshot of the LXC or not.
> So, it should be 1 *only* for the main branch of lxc. But in all stable version it is 0.

I understand that, but that's not my question. You explained how it is
intended to be used. But how is it actually used?

>
> > Have you analysed known reverse dependencies to understand the impact
> of making this change? What did you find?
>
> I have analyzed well-known reverse dependency go-lxc. It's used by LXD
> to communicate with liblxc C API.

Sure, but it is insufficient to consider just the reverse dependency
involved in the use case you're trying to fix. We must consider all
other reasonable use cases as well.

> Speaking honestly, I have no idea about other good ways to fix this. And
> this change seems to be a "minimal" for me because it does not change
> LXC code (and should not) it's just a matter of having proper build
> configuration.

The risk is that some other project is dependent on this variable in
some way that has not been considered and will break if the change is
made.

> I don't think that changing LXC_DEVEL to 0 can break any properly written code. For example, Debian folks have it disabled:
> https://git.launchpad.net/ubuntu/+source/lxc/tree/meson.build?h=applied/debian/bookworm#n36

For SRU purposes, it is not sufficient to rely on your "properly written
software" condition. We must also avoid regressing "improperly written
software" as much as we can in any change we make to a stable release.
In other words, "your software was written improperly and therefore the
fact that you, as a user, were regressed by our change to a stable
Ubuntu release is your problem and we don't care" would be an
unacceptable position to take if a user reported a regression as a
result of making this change.

> Of course, we can patch go-lxc (go-lxc also part of the LXC project).
> But this will be a hacky and incorrect way to fix things.

Ah, sorry, I had assumed that VERSION_AT_LEAST was being defined by the
lxc source package. Nevertheless, a less-than-clean fix is what we
sometimes need to do in stable releases to minimise regression risk to
users. It can be gated on a build against a specific version to keep the
solution clean for the future, though. For example, in your build system
you could check if you're

But this also suggests that there isn't actually a bug that impacts a
binary that is shipped by Ubuntu in Jammy

Please reconsider:

1) What use cases might be regressed as a result of this change, even
for software that is not "properly written". This is a hard requirement
for any s...

Read more...

Revision history for this message
Robie Basak (racb) wrote :

Sorry, I just hit send on a reply by accident on a reply before I was ready - I intended to postpone it instead. I need to go now but I will get back to this later and fix my reply.

Revision history for this message
Aleksandr Mikhalitsyn (mihalicyn) wrote : Re: liblxc-dev was built with LXC_DEVEL=1 in Ubuntu Jammy/Kinetic

>I meant the *Ubuntu* development release, not an upstream development
>release.

Ugh. If applied/ubuntu/devel is the right branch to check it then it is not fixed in the Ubuntu development release too.

See:
https://git.launchpad.net/ubuntu/+source/lxc/tree/meson.build?h=applied/ubuntu/devel#n33

At the same time in Debian:
https://git.launchpad.net/ubuntu/+source/lxc/tree/meson.build?h=applied/debian/bookworm#n36

>I understand that, but that's not my question. You explained how it is
>intended to be used. But how is it actually used?

It is used precisely as it intended to be used (at least in the go-lxc) :)

>Sure, but it is insufficient to consider just the reverse dependency
>involved in the use case you're trying to fix. We must consider all
>other reasonable use cases as well.

Ok, let's take https://github.com/search?q=LXC_DEVEL&type=code
As I can see from the search results there is no any other use cases for LXC_DEVEL
anywhere except go-lxc.

>For SRU purposes, it is not sufficient to rely on your "properly written
>software" condition. We must also avoid regressing "improperly written
>software" as much as we can in any change we make to a stable release.

Sure, I agree. But I'm 99.999% sure that this change is safe :)

>But this also suggests that there isn't actually a bug that impacts a
>binary that is shipped by Ubuntu in Jammy

It does not impacts a *binary*. But it impacts /usr/include/lxc/version.h file contents which is a
part of a liblxc-dev package.

>1) What use cases might be regressed as a result of this change, even
>for software that is not "properly written". This is a hard requirement
>for any stable release update in Ubuntu.

Have done using https://github.com/search?q=LXC_DEVEL&type=code

>2) In light of the above, what is an appropriate minimal way to fix the
>issue.

I believe that my fix is the minimal appropriate way to fix the issue.

Revision history for this message
Stéphane Graber (stgraber) wrote :

This was definitely a mistake made when preparing the original LXC 5.0 snapshot for upload in Ubuntu.

LXC_DEVEL=1 should only ever be set when dealing with current snapshots of the upstream codebase.
Shipping an older snapshot with LXC_DEVEL=1 set will cause any tool that consumes liblxc and which needs to do feature detection to incorrectly expect that the liblxc version present on the system has that feature supported, at best causing build failures, at worst causing runtime crashes.

I can certainly see how this mess was created with 5.0.0 as we had to push a pre-release snapshot to the archive and keep the build on autotools due to issues with meson at the time (resolved in 5.0.1).
Using an upstream snapshot rather than a release tarball, caused the inclusion of the problematic LXC_DEVEL=1.

What's quite puzzling is how we ended up with the 5.0.1 upload also having that LXC_DEVEL=1 bit be applied as a patch on top of it... Looking at the changelog, it appears that Serge simply pulled all changes following 5.0.1 from git, which he likely did mistakenly looking at the master branch rather than the stable-5.0 branch which wouldn't have had that particular change.

My opinion is that we really need to:
 - Drop the LXC_DEVEL=1 cherry-pick from noble, ideally merge with Debian to pull in the more current 5.0.3 too.
 - Drop the LXC_DEVEL=1 cherry-pick from both mantic and lunar
 - Add a batch to drop LXC_DEVEL=1 from the git snapshot of 5.0 that's in jammy

Additionally, I'd very strongly recommend that an autopkgtest test is added to validate that no liblxc package ever ship with LXC_DEVEL=1 ever again.

Revision history for this message
Serge Hallyn (serge-hallyn) wrote :

> Looking at the changelog, it appears that Serge simply pulled all changes following 5.0.1 from git, which he likely did mistakenly looking at the master branch rather than the stable-5.0 branch which wouldn't have had that particular change.

That sounds like exactly what I would do.

Revision history for this message
Robie Basak (racb) wrote :

OK, thanks all!

In that case, Aleksandr (or someone) please could you start by preparing a debdiff for noble to fix the issue there and add an appropriate autopkgtest as Stéphane recommends? While noble isn't open the SRU isn't strictly blocked on this, but we should at least have the upload ready to go.

Revision history for this message
Aleksandr Mikhalitsyn (mihalicyn) wrote :

Sure, I will do that.

Revision history for this message
Bryce Harrington (bryce) wrote :

[Unsubscribing sponsors pending resolution of Robie's request]

Simon Déziel (sdeziel)
summary: - liblxc-dev was built with LXC_DEVEL=1 in Ubuntu Jammy/Kinetic
+ liblxc-dev was built with LXC_DEVEL=1 in Ubuntu 22.04 and later releases
Revision history for this message
Aleksandr Mikhalitsyn (mihalicyn) wrote : Re: liblxc-dev was built with LXC_DEVEL=1 in Ubuntu 22.04 and later releases

Ok, I'm attaching a debdiff for Noble.

Changelog:
    Import LXC 5.0.3

    - imported LXC 5.0.3 original sources
    - dropped all debian/patches which are present in the LXC 5.0.3 already
    - added autopkgtest to ensure that LXC_DEVEL is always 0
    - aligned package names with the Debian ones:
     * lxc-utils and lxc1 are now transitional to lxc
     * lxc takes a place of lxc-utils and ships lxc-* utilities
     * liblxc-dev is now transitional to lxc-dev
     * lxc-dev takes a place of liblxc-dev and ships liblxc headers
     * upgrade path fixes by Simon Deziel

    Big thanks to Simon Deziel and Stéphane Graber for advices and help.

Tested by Simon Déziel and me using PPA:
https://launchpad.net/~mihalicyn/+archive/ubuntu/lxc-test-ppa

Git tree (both are equal):
https://git.launchpad.net/~mihalicyn/ubuntu/+source/lxc/log/?h=ubuntu/noble-devel
https://github.com/mihalicyn/lxc-pkg-ubuntu/commits/ubuntu/noble-devel

Revision history for this message
Aleksandr Mikhalitsyn (mihalicyn) wrote :

The ubuntu-sponsors team has been subscribed to the bug

Revision history for this message
Dave Jones (waveform) wrote :

Thanks very much for the updated debdiff. There's a lot of changes here to look at and unfortunately I've run out of time looking through it all during my patch-pilot shift, but here's what I've got so far:

The major thing that I think needs correction is that this patch is built on top of ubuntu/noble-devel by importing the upstream 5.0.3, but what Stéphane suggested in comment 14 was to take the Debian upstream (currently 5.0.3-2) and build on top of that. In this case, the commits in the repo would be based on debian/sid rather than ubuntu/noble-devel. This would ensure we incorporate the changes Debian has placed on top of lxc, as well as our own (and means in future we can follow the git-ubuntu merge process for this package).

The proposed changes also include package renames (which will be necessary if we're basing off the upstream Debian version). As far as I can tell they look sane but I haven't dug deep into this bit yet (it's something I always need to read the policy guides and manuals for!).

The requested autopkgtest to ensure LXC_DEVEL==0 is present, and looks good.

I have to dash to an appointment now, but I'll try and add some more notes later this evening.

Revision history for this message
Aleksandr Mikhalitsyn (mihalicyn) wrote :

Hello, Dave!

Huge thanks for your attention to this bug!

>The major thing that I think needs correction is that this patch is built on top of ubuntu/noble-devel by importing the upstream 5.0.3, but what Stéphane suggested in comment 14 was to take the Debian upstream (currently 5.0.3-2) and build on top of that.

Ah, yes. My bad. I guess that I can easily fix that. Thanks!

Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

Thanks, Aleksandr.

I am unsubscribing ubuntu-sponsors until https://bugs.launchpad.net/ubuntu/+source/lxc/+bug/2039873/comments/21 is addressed. Please subscribe it back here when it is done.

Revision history for this message
Aleksandr Mikhalitsyn (mihalicyn) wrote :

Ok, I have tried to do that but get stuck and have a few questions about the process.

>In this case, the commits in the repo would be based on debian/sid rather than ubuntu/noble-devel. This >would ensure we incorporate the changes Debian has placed on top of lxc, as well as our own (and means in >future we can follow the git-ubuntu merge process for this package).

Does this mean that I have to drop our (ubuntu-specific) changelog file and just replace it with the changelog file from the debian source package?

Even more, do I need to take only source code from debian source package or packaging metadata too (debian/ directory)?

I can see two options:
1. take debian/sid source package, unpack, add Ubuntu-specific patches, add a changelog entry, add transition packages entries to the debian/control file. In this case we loosing all the changelog from Ubuntu LXC package.

2. take debian/sid source package, unpack, drop debian/ directory and replace it with the debian/ directory from the Ubuntu package from ubuntu/noble-devel, ...

Which option is a correct and preferred one?

Revision history for this message
Lucas Kanashiro (lucaskanashiro) wrote :

Hi Aleksandr,

Let me try to clarify this for you. The two options presented by you are not what we want. We want to grab the package from Debian unstable (with the latest changes) and merge what we have in Ubuntu, which means having a complete changelog (including the previous Ubuntu changes). You can find some info about the merge process here:

https://wiki.ubuntu.com/UbuntuDevelopment/Merging

There is some outdated content there (like using bzr), if you want to do this using git properly you can try to use the workflow described here:

https://wiki.ubuntu.com/UbuntuDevelopment/Merging/GitWorkflow

This is also some good content maintained by the Canonical Server team which explains well the package merge process in our team's perspective:

https://github.com/canonical/ubuntu-maintainers-handbook/blob/main/PackageMerging.md

I hope that's useful.

Now, I took a quick look at the proposed changes and I believe you should double check the necessity of those binary packages name changes. We are providing some transitional packages since Jammy, maybe now it is time to get rid off them. In Debian, those transitional packages do not exist, a good a idea (since we are merging our changes with Debian again) might be to follow the package names from Debian. If it makes sense, this will make the package way simpler. As a data point, the Ubuntu package provides 12 binary packages and the Debian one provides 6 binary packages.

In the debian/tests/control file you need to add a comma (',') between the two restriction names. A comment here is that if you merge the changes from Debian we will get 2 more DEP-8 tests (autopkgtest) in the package.

Nitpick: you committed the debian/files file which is not necessary.

Revision history for this message
Aleksandr Mikhalitsyn (mihalicyn) wrote :

Dear colleagues,

I have taken a look on this ubuntu-import/rebase thing, thanks a lot for your suggestions and advice.

Unfortunately, I can't see any possibility to follow this Ubuntu-import way. Because the last Debian-based version of the LXC package was in 2012 (!) [ https://git.launchpad.net/ubuntu/+source/lxc/commit/?id=b0c54a19dda57a37294b244982bcfd425db8dbd8 ].

As I can see from https://github.com/canonical/ubuntu-maintainers-handbook/blob/main/PackageMerging.md#identify-logical-changes I have to go through *all* the git history from 2012 and split all the changes to a separate commits, then do a lot of other stuff. I can barely imagine doing all of that without making a single mistake in the middle which, effectively, will make all the rest of the a rebase job completely wrong (and I'll have to start everything from the beginning).

>In the debian/tests/control file you need to add a comma (',') between the two restriction names. A comment here is that if you merge the changes from Debian we will get 2 more DEP-8 tests (autopkgtest) in the package.

Thanks, will do.

>Nitpick: you committed the debian/files file which is not necessary.

Thanks, I'll remove it.

So, my question is, that if it's possible just to take a clean debian/sid branch, then merge *only* debian/changelog somehow and *manually* put all the necessary Ubuntu-specific patches on top without using all of this heavy machinery with rebasing and editing thousands of commits?

I understand that we have a very strict processes around importing packages, and I understand why. But it looks like this processes are suitable for regularly sync-able packages. Which is not the case for LXC, because most of LXC developers used to be a Canonical employees and Ubuntu LXC package was an independent from the Debian one (for 12 years!).

And another question is that if it would be possible to me to *only* fix the LXC_DEVEL issue without rebasing anything and updating anything to make things just work. Then one day, when we decide how to sort this rebase stuff out with such an deeply out-of-sync package I'll be happy to do a rebase on the debian/sid. I want to avoid Noble to be released with this LXC_DEVEL issue because it bothers developers/users of LXC/LXD.

Kind regards,
Alex

Revision history for this message
Aleksandr Mikhalitsyn (mihalicyn) wrote :

Alternatively, I can just pull a new upstream LXC sources and we keep the Ubuntu-specific package as it is without switching to a Debian base if it's so complex procedure. WDYT?

Revision history for this message
Stéphane Graber (stgraber) wrote :

My two cents here are that we should:
 - Rapidly fix the LXC_DEVEL situation across all supported Ubuntu releases.
 - Separately prepare a new package for noble which performs the Debian merge and introduces the needed transitional packages to get users from the current Ubuntu-specific naming over to the Debian names, those can then go away in noble+1 at which point LXC should be automatically syncable from Debian.

Revision history for this message
Thomas Parrott (tomparrott) wrote :

I agree with Stéphane, it would be better to get the LXC_DEVEL issue fixed quickly and then deal with the packaging refresh separately.

Revision history for this message
Aleksandr Mikhalitsyn (mihalicyn) wrote :

debdiff for mantic/noble (they have the same version 1:5.0.1-0ubuntu7 currently)

+lxc (1:5.0.1-0ubuntu8) mantic; urgency=medium
+
+ * Fix the LXC_DEVEL value to be 0
+ - d/p/0003-meson-Set-DEVEL-flag-post-release.patch was dropped
+ as it should not be in the production builds
+ * Added autopkgtest to ensure that LXC_DEVEL is always 0
+ - debian/tests/build: add "build" autopkgtest script
+ - debian/tests/control: declare "build" autopkgtest
+
+ -- Alexander Mikhalitsyn <email address hidden> Thu, 18 Jan 2024 16:20:47 +0100

Revision history for this message
Aleksandr Mikhalitsyn (mihalicyn) wrote :
Revision history for this message
Aleksandr Mikhalitsyn (mihalicyn) wrote :

Updated debdiff for Mantic/Noble (added Launchpad bug reference)

PPA:
https://launchpad.net/~mihalicyn/+archive/ubuntu/lxc-test-ppa-for-mantic-and-noble

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

I imported your noble debdiff and pushed to this branch:

https://code.launchpad.net/~ahasenack/ubuntu/+source/lxc/+git/lxc/+ref/noble-lxc-drop-devel-2039873

If you run "git ubuntu clone lxc; cd lxc; git ubuntu remote add ahasenack; git checkout noble-lxc-drop-devel-2039873" you should have a local copy. Or if using just git, not git-ubuntu, follow the instructions from the URL above.

Regarding the new d/t/build dep8 test:

"""
d/t/control:
Tests: build
Restrictions: allow-stderr superficial

d/t/build:
#!/bin/sh
# autopkgtest check: very basic checks to ensure that LXC package is in a good shape

set -eux

# ensure that LXC_DEVEL is 0
grep "LXC_DEVEL" meson.build | grep 0
"""

a) being marked as superficial won't block a migration if it fails (IIRC: at least, it's definitely not a hard error). And we do want this to "stop the line" if it fails, right?

b) it's checking something from the source code, which could be later changed via d/rules during build. What I mean is that this is not the place users will get the value of LXC_DEVEL from. We should be checking the include file that gets installed on the user's system, either via grep, or preferably a small C program that includes that version.h file and checks the value of LXC_DEVEL.

c) A DEP8 test is awesome, thanks for that. But it might too close to a release for such a check. Imagine an SRU gets sponsored, then reviewed, then accepted, and then we get the DEP8 result that says "oops, LXC_DEVEL=1 is leaking again". How about also checking it during package build? I won't block the upload on this, though, but it would be good to have I think, even if later.

What are your thoughts on (a) and (b)?

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Food for thought for an SRU, here is an example of an upstream that noted[1] this problem:

****NOTE:**** If you use the `liblxc-dev` package and get compile time errors when building the `go-lxc` module,
ensure that the value for `LXC_DEVEL` is `0` for your `liblxc` build. To check that, look at `/usr/include/lxc/version.h`.
If the `LXC_DEVEL` value is `1`, replace it with `0` to work around the problem. It's a packaging bug, and
we are aware of it for Ubuntu 22.04/22.10. Ubuntu 23.04/23.10 does not have this problem.

1. https://sources.debian.org/src/incus/0.4-1/doc/installing.md/?hl=192#L191

Changed in lxc (Ubuntu Mantic):
status: New → Confirmed
Changed in lxc (Ubuntu Jammy):
status: New → Confirmed
Revision history for this message
Simon Déziel (sdeziel) wrote :

In fact, the same but updated note was already on https://documentation.ubuntu.com/lxd/en/latest/installing/#install-lxd-from-source where it mentions that it affects 22.04 onward while the one you dug up says 23.04/23.10 are not affected ;)

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

I updated the tasks on this bug after having verified what has LXC_DEVEL set to 1, so we should be good now in terms of affected releases.

Revision history for this message
Aleksandr Mikhalitsyn (mihalicyn) wrote :

>What are your thoughts on (a) and (b)?

>being marked as superficial won't block a migration if it fails (IIRC: at least, it's definitely not a hard error). And we do want this to "stop the line" if it fails, right?

Ideally, it's better to block upload of package if this fails, yes. Because it's really bad to have LXC_DEVEL = 1 in the production release.

>b) it's checking something from the source code, which could be later changed via d/rules during build. What I mean is that this is not the place users will get the value of LXC_DEVEL from.

Yeah, ideally, yes. But I have just gone with the simplest possible solution to check this.

>A DEP8 test is awesome, thanks for that. But it might too close to a release for such a check. Imagine an SRU gets sponsored, then reviewed, then accepted, and then we get the DEP8 result that says "oops, LXC_DEVEL=1 is leaking again". How about also checking it during package build? I won't block the upload on this, though, but it would be good to have I think, even if later.

Ah, to be honest I was sure that this check will be performed before uploading this change to the Ubuntu package repositories. Can you give me a hint or send me to the manual about how to make this check during the package build?

>I won't block the upload on this, though, but it would be good to have I think, even if later.

Thanks for that. Yeah, I think that if everything seems generally fine to you to upload this as it is I can do this further improvements and modification in a later patches. As we anyways want (ideally) to rebase this on top of debian/sid.

Revision history for this message
Dave Jones (waveform) wrote :

I had a little time to look over this again in my patch-pilot shift. I agree it's almost certainly best to just get LXC_DEVEL set correctly quickly. Andreas' suggested branch is a good start, but I agree with the critique that the DEP-8 test should be checking the installed headers rather than the built source. Due to this, it's probably also best if the test is named something other than "build". Also, I agree it shouldn't be marked "superficial".

I've pushed a few commits on top of Andreas' to another clone here, which also fixes a build-failure on noble (lib/systemd instead of usr/lib/systemd in d/lxc-utils.install):

https://code.launchpad.net/~waveform/ubuntu/+source/lxc/+git/lxc

I've tested the DEP-8 passes with the branch, and crucially that it fails when the LXC_DEVEL fix is reverted (which it does). If this looks sane to someone else, I'm happy to sponsor this into noble (and start SRU proceedings for mantic and jammy).

Revision history for this message
Aleksandr Mikhalitsyn (mihalicyn) wrote :

Thanks, Dave!

Your branch and changes are looking good to me.

Revision history for this message
Dave Jones (waveform) wrote :

Okay, good enough for me -- I'll sponsor my aforementioned branch for noble and have a look at the mantic and jammy SRUs tomorrow, assuming everything builds happily in noble.

Revision history for this message
Dave Jones (waveform) wrote :

Noble's now in proposed and looks to have built successfully so I'll mark that Fix Committed. I'm just uploading the mantic and jammy SRUs (the jammy one took a little longer as the fix is slightly different there; appears the build system changed between jammy and mantic).

Changed in lxc (Ubuntu Noble):
status: Confirmed → Fix Committed
Dave Jones (waveform)
summary: - liblxc-dev was built with LXC_DEVEL=1 in Ubuntu 22.04 and later releases
+ [SRU] liblxc-dev was built with LXC_DEVEL=1 in Ubuntu 22.04 and later
+ releases
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package lxc - 1:5.0.1-0ubuntu8

---------------
lxc (1:5.0.1-0ubuntu8) noble; urgency=medium

  * Fix the LXC_DEVEL value to be 0 (LP: #2039873)
    - d/p/0003-meson-Set-DEVEL-flag-post-release.patch was dropped
      as it should not be in the production builds
  * Added autopkgtest to ensure that LXC_DEVEL is always 0
    - debian/tests/no-devel: add "no-devel" autopkgtest script
    - debian/tests/control: declare "no-devel" autopkgtest
  * d/lxc-utils.install: Fixed lib/systemd path (to usr/lib/systemd)

 -- Alexander Mikhalitsyn <email address hidden> Thu, 18 Jan 2024 16:20:47 +0100

Changed in lxc (Ubuntu Noble):
status: Fix Committed → Fix Released
Revision history for this message
Aleksandr Mikhalitsyn (mihalicyn) wrote :

Huge thanks, Dave!

Revision history for this message
Timo Aaltonen (tjaalton) wrote : Please test proposed package

Hello Aleksandr, or anyone else affected,

Accepted lxc into mantic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/lxc/1:5.0.1-0ubuntu8~23.10.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-mantic to verification-done-mantic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-mantic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in lxc (Ubuntu Mantic):
status: Confirmed → Fix Committed
tags: added: verification-needed verification-needed-mantic
Revision history for this message
Timo Aaltonen (tjaalton) wrote :

The jammy upstream version is at 5.0.0~git2209-g5a7b9ce67, which has devel written all over it? Why should it drop LXC_DEVEL?

Revision history for this message
Aleksandr Mikhalitsyn (mihalicyn) wrote :

Because, this is a production build of package. LXC_DEVEL is about type of build not about version.
For any package builds we should have LXC_DEVEL=0. The only case when LXC_DEVEL=1 is when you are doing local development of the LXC and building it for your self, or you building a package to install it on the CI VMs for testing purposes. Because whan LXC_DEVEL does it it effectively disables all the VERSION_AT_LEAST checks and make all of them to pass. Which is absolutely incorrect for any production builds.

Revision history for this message
Timo Aaltonen (tjaalton) wrote :

Hello Aleksandr, or anyone else affected,

Accepted lxc into jammy-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/lxc/1:5.0.0~git2209-g5a7b9ce67-0ubuntu1.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-jammy to verification-done-jammy. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-jammy. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in lxc (Ubuntu Jammy):
status: Confirmed → Fix Committed
tags: added: verification-needed-jammy
Revision history for this message
Simon Déziel (sdeziel) wrote :

This was verified on Jammy with -proposed (1:5.0.0~git2209-g5a7b9ce67-0ubuntu1.1). Here's how:

$ lxc launch ubuntu-daily:22.04 c1
Creating c1
Starting c1

root@c1:~# apt-get update
...
root@c1:~# apt-get install -Vy liblxc-dev
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
The following additional packages will be installed:
   libfuse2 (2.9.9-5ubuntu3)
   liblxc-common (1:5.0.0~git2209-g5a7b9ce67-0ubuntu1)
   liblxc1 (1:5.0.0~git2209-g5a7b9ce67-0ubuntu1)
   lxcfs (5.0.0-0ubuntu2)
   uidmap (1:4.8.1-2ubuntu2.1)
The following NEW packages will be installed:
   libfuse2 (2.9.9-5ubuntu3)
   liblxc-common (1:5.0.0~git2209-g5a7b9ce67-0ubuntu1)
   liblxc-dev (1:5.0.0~git2209-g5a7b9ce67-0ubuntu1)
   liblxc1 (1:5.0.0~git2209-g5a7b9ce67-0ubuntu1)
   lxcfs (5.0.0-0ubuntu2)
   uidmap (1:4.8.1-2ubuntu2.1)
0 upgraded, 6 newly installed, 0 to remove and 0 not upgraded.
Need to get 1462 kB of archives.
...

root@c1:~# dpkg -l | grep liblxc-dev
ii liblxc-dev 1:5.0.0~git2209-g5a7b9ce67-0ubuntu1 amd64 Linux Containers userspace tools (development)
root@c1:~# grep LXC_DEVEL /usr/include/lxc/version.h
#define LXC_DEVEL 1

# enable -proposed

root@c1:~# apt-get update
...
root@c1:~# apt-get install -Vy liblxc-dev
...
The following packages will be upgraded:
   liblxc-common (1:5.0.0~git2209-g5a7b9ce67-0ubuntu1 => 1:5.0.0~git2209-g5a7b9ce67-0ubuntu1.1)
   liblxc-dev (1:5.0.0~git2209-g5a7b9ce67-0ubuntu1 => 1:5.0.0~git2209-g5a7b9ce67-0ubuntu1.1)
   liblxc1 (1:5.0.0~git2209-g5a7b9ce67-0ubuntu1 => 1:5.0.0~git2209-g5a7b9ce67-0ubuntu1.1)
3 upgraded, 0 newly installed, 0 to remove and 32 not upgraded.
Need to get 1268 kB of archives.
...

root@c1:~# grep LXC_DEVEL /usr/include/lxc/version.h
#define LXC_DEVEL 0

tags: added: verification-done-jammy
removed: verification-needed-jammy
Revision history for this message
Simon Déziel (sdeziel) wrote :

This was verified on Mantic with -proposed (1:5.0.1-0ubuntu8~23.10.1). Here's how:

$ lxc launch ubuntu-daily:23.10 c2
Creating c2
Starting c2

$ lxc shell c2

root@c2:~# apt-get update
...
root@c2:~# apt-get install -Vy liblxc-dev
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
The following additional packages will be installed:
...
   liblxc-common (1:5.0.1-0ubuntu7)
   liblxc1 (1:5.0.1-0ubuntu7)
...
   lxcfs (5.0.3-1)
..

root@c2:~# dpkg -l | grep liblxc-dev
ii liblxc-dev 1:5.0.1-0ubuntu7 amd64 Linux Containers userspace tools (development)
root@c2:~# grep LXC_DEVEL /usr/include/lxc/version.h
#define LXC_DEVEL 1

# enable -proposed

root@c2:~# apt-get update
...
root@c2:~# apt-get install -t mantic-proposed -Vy liblxc-dev
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
The following additional packages will be installed:
   liblxc-common (1:5.0.1-0ubuntu8~23.10.1)
   liblxc1 (1:5.0.1-0ubuntu8~23.10.1)
The following packages will be upgraded:
   liblxc-common (1:5.0.1-0ubuntu7 => 1:5.0.1-0ubuntu8~23.10.1)
   liblxc-dev (1:5.0.1-0ubuntu7 => 1:5.0.1-0ubuntu8~23.10.1)
   liblxc1 (1:5.0.1-0ubuntu7 => 1:5.0.1-0ubuntu8~23.10.1)
3 upgraded, 0 newly installed, 0 to remove and 5 not upgraded.
Need to get 1804 kB of archives.
...

root@c2:~# grep LXC_DEVEL /usr/include/lxc/version.h
#define LXC_DEVEL 0

tags: added: verification-done verification-done-mantic
removed: verification-needed verification-needed-mantic
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package lxc - 1:5.0.1-0ubuntu8~23.10.1

---------------
lxc (1:5.0.1-0ubuntu8~23.10.1) mantic; urgency=medium

  [ Alexander Mikhalitsyn ]
  * Fix the LXC_DEVEL value to be 0 (LP: #2039873)
    - d/p/0003-meson-Set-DEVEL-flag-post-release.patch was dropped
      as it should not be in the production builds
  * Added autopkgtest to ensure that LXC_DEVEL is always 0
    - debian/tests/no-devel: add "no-devel" autopkgtest script
    - debian/tests/control: declare "no-devel" autopkgtest

 -- Dave Jones <email address hidden> Wed, 24 Jan 2024 16:25:47 +0000

Changed in lxc (Ubuntu Mantic):
status: Fix Committed → Fix Released
Revision history for this message
Łukasz Zemczak (sil2100) wrote : Update Released

The verification of the Stable Release Update for lxc has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package lxc - 1:5.0.0~git2209-g5a7b9ce67-0ubuntu1.1

---------------
lxc (1:5.0.0~git2209-g5a7b9ce67-0ubuntu1.1) jammy; urgency=medium

  [ Dave Jones ]
  * Fix the LXC_DEVEL value to be 0 (LP: #2039873)
    - d/p/0002-Set-DEVEL-flag-post-release.patch added to force this
      value in the autoconf script

  [ Alexander Mikhalitsyn ]
  * Added autopkgtest to ensure that LXC_DEVEL is always 0
    - debian/tests/no-devel: add "no-devel" autopkgtest script
    - debian/tests/control: declare "no-devel" autopkgtest

 -- Dave Jones <email address hidden> Wed, 24 Jan 2024 16:26:06 +0000

Changed in lxc (Ubuntu Jammy):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.