liblxc-dev was built with LXC_DEVEL=1 in Ubuntu Jammy/Kinetic

Bug #2039873 reported by Aleksandr Mikhalitsyn
20
This bug affects 2 people
Affects Status Importance Assigned to Milestone
lxc (Ubuntu)
Confirmed
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 ]

-

Tags: patch
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 :

>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]

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.