[PATCH 1/3] dpkg-base: Lock do_adjust_git against each other

15 views
Skip to first unread message

Jan Kiszka

unread,
Aug 12, 2021, 5:23:31 AM8/12/21
to isar-users, Uladzimir Bely, Henning Schild
From: Jan Kiszka <jan.k...@siemens.com>

This is needed since e47ada143469 because we now adjust the links to
mirrors in DL_DIR which may collide with other tasks doing this at the
same time.

Locking with also be needed soon for establishing the host-side link
to the download dir.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---
meta/classes/dpkg-base.bbclass | 1 +
1 file changed, 1 insertion(+)

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index ec8fbc1..fe16846 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -55,6 +55,7 @@ python do_adjust_git() {
}

addtask adjust_git before do_dpkg_build
+do_adjust_git[lockfiles] += "${DL_DIR}/isar.lock"

inherit patch
addtask patch before do_adjust_git
--
2.31.1

Jan Kiszka

unread,
Aug 12, 2021, 5:23:31 AM8/12/21
to isar-users, Uladzimir Bely, Henning Schild
Finally allow the adjusted git repos work both inside and outside the
buildchroots. Also fixes the race introduced by patching links in
DL_DIR.

NOTE: Not yet fully tested, our CI is busy with that now but just
stumbled again over the broken umount patches.

Jan

Jan Kiszka (3):
dpkg-base: Lock do_adjust_git against each other
dpkg-base: Make mirror link relative
Rework do_adjust_git to support inside and outside usage

meta/classes/dpkg-base.bbclass | 15 ++++++++++++---
meta/conf/bitbake.conf | 1 +
meta/recipes-devtools/buildchroot/buildchroot.inc | 2 ++
3 files changed, 15 insertions(+), 3 deletions(-)

--
2.31.1

Jan Kiszka

unread,
Aug 12, 2021, 5:23:32 AM8/12/21
to isar-users, Uladzimir Bely, Henning Schild
From: Jan Kiszka <jan.k...@siemens.com>

Rewrite the alternates entry to use a relative path outside of the
workdir. That linke can then be created independently of the outer build
environment as well as inside the buildchroot.

We use ${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/.git-downloads outside
and /home/.git-downloads inside the buildchroot.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---
meta/classes/dpkg-base.bbclass | 8 +++++++-
meta/conf/bitbake.conf | 1 +
meta/recipes-devtools/buildchroot/buildchroot.inc | 2 ++
3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index a4c8663..cd6c649 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -19,6 +19,12 @@ python do_adjust_git() {

rootdir = d.getVar('WORKDIR', True)

+ git_link = os.path.join(d.getVar('GIT_DL_LINK_DIR'), '.git-downloads')
+ git_dl = os.path.join(d.getVar("DL_DIR"), "git")
+
+ if not os.path.exists(git_link) or os.path.realpath(git_link) != git_dl:
+ os.symlink(git_dl, git_link)
+
for src_uri in (d.getVar("SRC_URI", True) or "").split():
try:
fetcher = bb.fetch2.Fetch([src_uri], d)
@@ -48,7 +54,7 @@ python do_adjust_git() {

if os.path.exists(alternates):
cmd = ["sed", "-i", alternates, "-e",
- "s|{}|/downloads|".format(d.getVar("DL_DIR"))]
+ "s|{}|../../../../../.git-downloads|".format(git_dl)]
bb.note(' '.join(cmd))
if subprocess.call(cmd) != 0:
bb.fatal("git alternates adjustment failed")
diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
index 9859456..7f5901d 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -48,6 +48,7 @@ GITPKGV = "${@bb.fetch2.get_srcrev(d, 'gitpkgv_revision')}"

# isar specific config
WORKDIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/${PN}/${PV}-${PR}"
+GIT_DL_LINK_DIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}"
DEPLOY_DIR_BOOTSTRAP = "${DEPLOY_DIR}/bootstrap"
DEPLOY_DIR_BUILDCHROOT = "${DEPLOY_DIR}/buildchroot"
DEPLOY_DIR_SDKCHROOT = "${DEPLOY_DIR}/sdkchroot"
diff --git a/meta/recipes-devtools/buildchroot/buildchroot.inc b/meta/recipes-devtools/buildchroot/buildchroot.inc
index 0c1fb7f..52a5f1c 100644
--- a/meta/recipes-devtools/buildchroot/buildchroot.inc
+++ b/meta/recipes-devtools/buildchroot/buildchroot.inc
@@ -67,6 +67,8 @@ buildchroot_install_files() {
sudo install -m 755 ${WORKDIR}/common.sh ${BUILDCHROOT_DIR}/isar/
sudo install -m 755 ${WORKDIR}/deps.sh ${BUILDCHROOT_DIR}/isar/

+ sudo ln -s /downloads/git "${BUILDCHROOT_DIR}/home/.git-downloads"
+
# Configure root filesystem
sudo install -m 755 ${WORKDIR}/configscript.sh ${BUILDCHROOT_DIR}
USER_ID=$(id -u)
--
2.31.1

Jan Kiszka

unread,
Aug 12, 2021, 6:31:38 AM8/12/21
to isar-users, Uladzimir Bely, Henning Schild
Hmm, this does not yet account for the case that the repo is placed in
some subdir under workdir. Will make this more general.

I also forgot to fix the task dependencies - devshell is not pulling
adjust_git.

Jan
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

Jan Kiszka

unread,
Aug 12, 2021, 7:48:54 AM8/12/21
to isar-users, Uladzimir Bely, Henning Schild
Finally allow the adjusted git repos work both inside and outside the
buildchroots. Also fixes the race introduced by patching links in
DL_DIR.

Changes in v2:
- build relative alternates path so that destsuffix paths are respected
- fix task dependencies so that devshell gets adjusted git again

NOTE: Still not yet fully tested, fast CI takes 3:30h (plus retries).

Jan

Jan Kiszka (4):
dpkg-base: Lock do_adjust_git against each other
dpkg-base: Make mirror link relative
Rework do_adjust_git to support inside and outside usage
Revert "dpkg: adjust task order to allow using "git" for patching"

meta/classes/dpkg-base.bbclass | 22 ++++++++++++++-----
meta/conf/bitbake.conf | 1 +
.../buildchroot/buildchroot.inc | 2 ++
3 files changed, 20 insertions(+), 5 deletions(-)

--
2.31.1

Jan Kiszka

unread,
Aug 12, 2021, 7:48:55 AM8/12/21
to isar-users, Uladzimir Bely, Henning Schild
From: Jan Kiszka <jan.k...@siemens.com>

Rewrite the alternates entry to use a relative path outside of the
workdir. That linke can then be created independently of the outer build
environment as well as inside the buildchroot.

We use ${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/.git-downloads outside
and /home/.git-downloads inside the buildchroot.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---
meta/classes/dpkg-base.bbclass | 11 ++++++++++-
meta/conf/bitbake.conf | 1 +
meta/recipes-devtools/buildchroot/buildchroot.inc | 2 ++
3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index a4c8663..992f55a 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -19,6 +19,12 @@ python do_adjust_git() {

rootdir = d.getVar('WORKDIR', True)

+ git_link = os.path.join(d.getVar('GIT_DL_LINK_DIR'), '.git-downloads')
+ git_dl = os.path.join(d.getVar("DL_DIR"), "git")
+
+ if not os.path.exists(git_link) or os.path.realpath(git_link) != git_dl:
+ os.symlink(git_dl, git_link)
+
for src_uri in (d.getVar("SRC_URI", True) or "").split():
try:
fetcher = bb.fetch2.Fetch([src_uri], d)
@@ -44,11 +50,14 @@ python do_adjust_git() {
destsuffix = ud.parm.get("destsuffix", def_destsuffix)
destdir = ud.destdir = os.path.join(rootdir, destsuffix)

+ git_link_rel = os.path.relpath(git_link,
+ os.path.join(destdir, ".git/objects"))
+
alternates = os.path.join(destdir, ".git/objects/info/alternates")

if os.path.exists(alternates):
cmd = ["sed", "-i", alternates, "-e",
- "s|{}|/downloads|".format(d.getVar("DL_DIR"))]
+ "s|{}|{}|".format(git_dl, git_link_rel)]
2.31.1

Jan Kiszka

unread,
Aug 12, 2021, 7:48:55 AM8/12/21
to isar-users, Uladzimir Bely, Henning Schild
From: Jan Kiszka <jan.k...@siemens.com>

This is needed since e47ada143469 because we now adjust the links to
mirrors in DL_DIR which may collide with other tasks doing this at the
same time.

Locking with also be needed soon for establishing the host-side link
to the download dir.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---
meta/classes/dpkg-base.bbclass | 1 +
1 file changed, 1 insertion(+)

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index ec8fbc1..fe16846 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass

Jan Kiszka

unread,
Aug 12, 2021, 7:48:55 AM8/12/21
to isar-users, Uladzimir Bely, Henning Schild
From: Jan Kiszka <jan.k...@siemens.com>

This will allow to use it both from the outer builder context and inside
the buildchroot. No idea why bitbake doesn't to that in the first place.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---
meta/classes/dpkg-base.bbclass | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index fe16846..a4c8663 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -28,8 +28,10 @@ python do_adjust_git() {

if os.path.islink(ud.localpath):
realpath = os.path.realpath(ud.localpath)
- if realpath.startswith(d.getVar("DL_DIR")):
- link = realpath.replace(d.getVar("DL_DIR"), '/downloads', 1)
+ filter_out = os.path.join(d.getVar("DL_DIR"), "git") + "/"
+ if realpath.startswith(filter_out):
+ # make the link relative
+ link = realpath.replace(filter_out, '', 1)
os.unlink(ud.localpath)
os.symlink(link, ud.localpath)

--
2.31.1

Jan Kiszka

unread,
Aug 12, 2021, 7:48:55 AM8/12/21
to isar-users, Uladzimir Bely, Henning Schild
From: Jan Kiszka <jan.k...@siemens.com>

This reverts commit b4c33ba69ed89849ecf32d8731aa97ecf88226f5.

No longer needed now that adjust_git stopped breaking the outer
environment. Moreover, this was broken /wrt devshell anyway.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---
meta/classes/dpkg-base.bbclass | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index 992f55a..ecc583c 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -65,11 +65,11 @@ python do_adjust_git() {
bb.fatal(str(e))
}

-addtask adjust_git before do_dpkg_build
+addtask adjust_git after do_unpack before do_patch
do_adjust_git[lockfiles] += "${DL_DIR}/isar.lock"

inherit patch
-addtask patch before do_adjust_git
+addtask patch after do_adjust_git before do_dpkg_build

SRC_APT ?= ""

--
2.31.1

Uladzimir Bely

unread,
Aug 13, 2021, 4:31:47 AM8/13/21
to isar-users
I've checked your series in CI (fast run) and it was OK.

Actually, the problem with cowsay rebuild appeared on some manual work with ISAR, when some changes in configuration were done and rebuild was started on top of previous build. So, the problem was reproducable only in such manual conditions.

I'm going to add some test to CI that will reproduce these conditions so that we would be sure the fix is complete.

Jan Kiszka

unread,
Aug 13, 2021, 6:13:08 AM8/13/21
to Uladzimir Bely, isar-users
On 13.08.21 10:31, Uladzimir Bely wrote:
> I've checked your series in CI (fast run) and it was OK.
>
> Actually, the problem with cowsay rebuild appeared on some manual work
> with ISAR, when some changes in configuration were done and rebuild was
> started on top of previous build. So, the problem was reproducable only
> in such manual conditions.
>
> I'm going to add some test to CI that will reproduce these conditions so
> that we would be sure the fix is complete.
>

Great, thanks in advance!

Jan

> On Thursday, August 12, 2021 at 2:48:54 PM UTC+3 Jan Kiszka wrote:
>
> Finally allow the adjusted git repos work both inside and outside the
> buildchroots. Also fixes the race introduced by patching links in
> DL_DIR.
>
> Changes in v2:
> - build relative alternates path so that destsuffix paths are respected
> - fix task dependencies so that devshell gets adjusted git again
>
> NOTE: Still not yet fully tested, fast CI takes 3:30h (plus retries).
>
> Jan
>
> Jan Kiszka (4):
> dpkg-base: Lock do_adjust_git against each other
> dpkg-base: Make mirror link relative
> Rework do_adjust_git to support inside and outside usage
> Revert "dpkg: adjust task order to allow using "git" for patching"
>
> meta/classes/dpkg-base.bbclass | 22 ++++++++++++++-----
> meta/conf/bitbake.conf | 1 +
> .../buildchroot/buildchroot.inc | 2 ++
> 3 files changed, 20 insertions(+), 5 deletions(-)
>
> --
> 2.31.1
>
> --
> You received this message because you are subscribed to the Google
> Groups "isar-users" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to isar-users+...@googlegroups.com
> <mailto:isar-users+...@googlegroups.com>.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/isar-users/3710f21b-429e-40c2-8a8f-814bf1b734c2n%40googlegroups.com
> <https://groups.google.com/d/msgid/isar-users/3710f21b-429e-40c2-8a8f-814bf1b734c2n%40googlegroups.com?utm_medium=email&utm_source=footer>.

Uladzimir Bely

unread,
Aug 13, 2021, 8:23:45 AM8/13/21
to isar-users
In addition to this series we still need something like I had done in "[RFC,2/2] dpkg-gdb: Reset git to SRCREV revision before patching".

Because the problem with git has gone, but there is a problem that we try apply patch on top of git tree where it's already applied.

Steps to reproduce are quite simple:

- Run first build: bitbake mc:qemuamd64-buster:isar-image-base
- Modify local.conf (e.g. set ISAR_USE_CACHED_BASE_REPO ?= "1" or do something else that will cause packages rebuild)
- Run second build: bitbake mc:qemuamd64-buster:isar-image-base

It ends up with:

ERROR: mc:qemuamd64-buster:cowsay-git-r0 do_patch: Applying 'isar.patch' failed:
The next patch would create the file debian/patches/isar.patch,
which already exists!  Assume -R? [n]

In CI tests we remove build/tmp, so that we don't face this problem.

Jan Kiszka

unread,
Aug 13, 2021, 8:28:45 AM8/13/21
to Uladzimir Bely, isar-users
On 13.08.21 14:23, Uladzimir Bely wrote:
> In addition to this series we still need something like I had done in
> "[RFC,2/2] dpkg-gdb: Reset git to SRCREV revision before patching".
>
> Because the problem with git has gone, but there is a problem that we
> try apply patch on top of git tree where it's already applied.
>
> Steps to reproduce are quite simple:
>
> - Run first build: bitbake mc:qemuamd64-buster:isar-image-base
> - Modify local.conf (e.g. set ISAR_USE_CACHED_BASE_REPO ?= "1" or do
> something else that will cause packages rebuild)
> - Run second build: bitbake mc:qemuamd64-buster:isar-image-base
>
> It ends up with:
>
> ERROR:mc:qemuamd64-buster:cowsay-git-r0 do_patch: Applying 'isar.patch'
> failed:
> The next patch would create the file debian/patches/isar.patch,
> which already exists!  Assume -R? [n]
>
> In CI tests we remove build/tmp, so that we don't face this problem.
>

This is supposed to be handled by the patch class - when it works
properly. Please check if quilt-based patching addresses this and only
git is affected and, if yes, also cross-check how that works in OE. We
either miss something from OE while using their patch class or there is
actually an upstream issue.

Jan

Uladzimir Bely

unread,
Aug 13, 2021, 8:40:43 AM8/13/21
to isar-users
When I previously looked at patch.bbclass, I noted, that we don't use it completely.

There are some patch_task_patch_prefunc() and patch_task_postfunc() here that are used in OE when PATCHTOOL='git'. But in our case they are never run due to not using PATCH_COMMIT_FUNCTIONS in isar.

I suppose (but can't be completely sure without deep look to OE) that prefunc may fix this 'patch is allready applied' issue in OE.

Henning Schild

unread,
Aug 13, 2021, 9:55:34 AM8/13/21
to Jan Kiszka, isar-users, Uladzimir Bely
Am Thu, 12 Aug 2021 13:48:49 +0200
schrieb Jan Kiszka <jan.k...@siemens.com>:
The name of the file and the location suggest pretty wide locking ...
could sound like the whole DL_DIR is locked, by isar ...

I know that is a "pattern" but not sure it is a good one. Maybe using
the task name would be better.

Henning

Henning Schild

unread,
Aug 13, 2021, 10:04:36 AM8/13/21
to Jan Kiszka, isar-users, Uladzimir Bely
Am Thu, 12 Aug 2021 13:48:50 +0200
schrieb Jan Kiszka <jan.k...@siemens.com>:

> From: Jan Kiszka <jan.k...@siemens.com>
>
> This will allow to use it both from the outer builder context and
> inside the buildchroot. No idea why bitbake doesn't to that in the
> first place.

It is git deciding to take absolute instead of relative ...

I just read the man-page and it seems that we can

git repack -a
rm .git/objects/info/alternates

to "unshare" a clone. That might have the mentioned storage and speed
issues on huge repos ... but would imho be cleaner and much less hacky
than missing with the alternates no matter if we mess relative or
absolute.

Henning

Henning Schild

unread,
Aug 13, 2021, 1:49:48 PM8/13/21
to isar-users, Jan Kiszka, Henning Schild
Instead of messing with the alternate make the clone not "shared"
anymore. Doing that will allow us to use git as a patchtool, also in
incremental rebuilds where do_patch might run again after do_adjust_git

It will also make sure git repos will be usable in all three
environments. On a container host, inside the container, and inside the
chroot ... as apposed to only the chroot in the end.

Signed-off-by: Henning Schild <henning...@siemens.com>
---
meta/classes/dpkg-base.bbclass | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index ec8fbc14d5e4..2a8e3ef4b817 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -45,11 +45,11 @@ python do_adjust_git() {
alternates = os.path.join(destdir, ".git/objects/info/alternates")

if os.path.exists(alternates):
- cmd = ["sed", "-i", alternates, "-e",
- "s|{}|/downloads|".format(d.getVar("DL_DIR"))]
+ cmd = ["git", "-C", destdir, "repack", "-a" ]
bb.note(' '.join(cmd))
if subprocess.call(cmd) != 0:
- bb.fatal("git alternates adjustment failed")
+ bb.fatal("git repack failed")
+ os.remove(alternates)
except bb.fetch2.BBFetchException as e:
bb.fatal(str(e))
}
@@ -57,7 +57,6 @@ python do_adjust_git() {
addtask adjust_git before do_dpkg_build

inherit patch
-addtask patch before do_adjust_git

SRC_APT ?= ""

--
2.31.1

Henning Schild

unread,
Aug 13, 2021, 4:13:35 PM8/13/21
to isar-users, Jan Kiszka, Henning Schild
Instead of messing with the alternate make the clone not "shared"
anymore. Doing that will allow us to use git as a patchtool, also in
incremental rebuilds where do_patch might run again after do_adjust_git

It will also make sure git repos will be usable in all three
environments. On a container host, inside the container, and inside the
chroot ... as apposed to only the chroot in the end.

Signed-off-by: Henning Schild <henning...@siemens.com>
---
meta/classes/dpkg-base.bbclass | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index ec8fbc14d5e4..82a4b39f1001 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -45,19 +45,18 @@ python do_adjust_git() {
alternates = os.path.join(destdir, ".git/objects/info/alternates")

if os.path.exists(alternates):
- cmd = ["sed", "-i", alternates, "-e",
- "s|{}|/downloads|".format(d.getVar("DL_DIR"))]
+ cmd = ["git", "-C", destdir, "repack", "-a" ]
bb.note(' '.join(cmd))
if subprocess.call(cmd) != 0:
- bb.fatal("git alternates adjustment failed")
+ bb.fatal("git repack failed")
+ os.remove(alternates)
except bb.fetch2.BBFetchException as e:
bb.fatal(str(e))
}

-addtask adjust_git before do_dpkg_build
+addtask adjust_git after do_unpack before do_dpkg_build

Henning Schild

unread,
Aug 13, 2021, 4:14:54 PM8/13/21
to isar-users, Jan Kiszka
Am Fri, 13 Aug 2021 22:13:31 +0200
schrieb Henning Schild <henning...@siemens.com>:
This is the change to v1.

Henning

Jan Kiszka

unread,
Aug 15, 2021, 2:55:37 PM8/15/21
to Henning Schild, isar-users
NACK, we have a pattern now that avoids this.

Jan Kiszka

unread,
Aug 15, 2021, 2:55:44 PM8/15/21
to Henning Schild, isar-users, Uladzimir Bely
Yeah, could move the lock into ${DL_DIR}/git.

Jan Kiszka

unread,
Aug 15, 2021, 3:02:27 PM8/15/21
to Uladzimir Bely, isar-users
On 13.08.21 14:40, Uladzimir Bely wrote:
> When I previously looked at patch.bbclass, I noted, that we don't use it
> completely.
>
> There are some patch_task_patch_prefunc() and patch_task_postfunc() here
> that are used in OE when PATCHTOOL='git'. But in our case they are never
> run due to not using PATCH_COMMIT_FUNCTIONS in isar.
>
> I suppose (but can't be completely sure without deep look to OE) that
> prefunc may fix this 'patch is allready applied' issue in OE.
>

If you can still reproduce the issue, just set PATCH_COMMIT_FUNCTIONS=1
to check if it has any impact.

Browsing poky and OE-core, I only find devtool setting this var. But
both aren't using git as patch tool anyway, a downstream layer/recipe
needs to requests that.

Uladzimir Bely

unread,
Aug 16, 2021, 3:53:03 AM8/16/21
to isar-users, Jan Kiszka
In the email from воскресенье, 15 августа 2021 г. 22:02:25 +03 user Jan Kiszka
wrote:
Yes, I also tried this way.

Actually, enabling PATCH_COMMIT_FUNCTIONS also forces to make some more
changes in patch.bbclass (like sys.path.insert for OE lib path) that makes the
difference increase.

Anyway, even if this set to "1", the problem of patch reapply is still here -
custom patch is going to be applied despite it was already done on previous
build. So, reset to SRC_REV is still required.

--
Uladzimir Bely
Promwad Ltd.
External service provider of ilbers GmbH
Maria-Merian-Str. 8
85521 Ottobrunn, Germany
+49 (89) 122 67 24-0
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov


Jan Kiszka

unread,
Aug 16, 2021, 3:55:33 AM8/16/21
to Uladzimir Bely, isar-users
On 16.08.21 09:52, Uladzimir Bely wrote:
> In the email from воскресенье, 15 августа 2021 г. 22:02:25 +03 user Jan Kiszka
> wrote:
>> On 13.08.21 14:40, Uladzimir Bely wrote:
>>> When I previously looked at patch.bbclass, I noted, that we don't use it
>>> completely.
>>>
>>> There are some patch_task_patch_prefunc() and patch_task_postfunc() here
>>> that are used in OE when PATCHTOOL='git'. But in our case they are never
>>> run due to not using PATCH_COMMIT_FUNCTIONS in isar.
>>>
>>> I suppose (but can't be completely sure without deep look to OE) that
>>> prefunc may fix this 'patch is allready applied' issue in OE.
>>
>> If you can still reproduce the issue, just set PATCH_COMMIT_FUNCTIONS=1
>> to check if it has any impact.
>>
>> Browsing poky and OE-core, I only find devtool setting this var. But
>> both aren't using git as patch tool anyway, a downstream layer/recipe
>> needs to requests that.
>>
>> Jan
>
> Yes, I also tried this way.
>
> Actually, enabling PATCH_COMMIT_FUNCTIONS also forces to make some more
> changes in patch.bbclass (like sys.path.insert for OE lib path) that makes the
> difference increase.
>
> Anyway, even if this set to "1", the problem of patch reapply is still here -
> custom patch is going to be applied despite it was already done on previous
> build. So, reset to SRC_REV is still required.
>

Strange. Did you try the same procedure in an OE setup, to compare what
happens there?

Uladzimir Bely

unread,
Aug 16, 2021, 4:27:19 AM8/16/21
to isar-users, Jan Kiszka
In the email from понедельник, 16 августа 2021 г. 10:55:31 +03 user Jan Kiszka
Not yet, while it's expected to take much time to compile.

Anyway, to catch the similar error, I need to find some package to rebuild in
Yocto/OE that follows the approach similar to one isar's cowsay uses: a patch
used by recipe that adds new patch to debian series.

Jan Kiszka

unread,
Aug 16, 2021, 6:45:15 AM8/16/21
to Uladzimir Bely, isar-users
Nope, this is unrelated. You just need to find a recipe that adds a
patch, then switch to PATCHTOOL=git, and finally trigger the rebuild in
a way that patch is re-run. That should be doable in OE without building
a complete system. In fact, you only need to force-run patch for a
specific recipe twice.

Jan Kiszka

unread,
Aug 16, 2021, 7:01:47 AM8/16/21
to Uladzimir Bely, isar-users
In fact, I'm not even sure we have a problem here. This is what I just
did over my patch queue:

bitbake mc:qemuarm64-buster:cowsay
edit isar.patch
bitbake mc:qemuarm64-buster:cowsay

And that succeeded, thus do_patch did a proper revert before trying to
apply the updated patch again.

How exactly do you trigger the problem you still see?

Henning Schild

unread,
Aug 16, 2021, 7:26:09 AM8/16/21
to Jan Kiszka, Uladzimir Bely, isar-users
Am Mon, 16 Aug 2021 13:01:43 +0200
schrieb Jan Kiszka <jan.k...@siemens.com>:
I bet the problem has to do with adjust_git. If do_patch runs after
adjust_git it will not work. Not be able to revert because git ops will
be broken.

If you edit isar.patch you rerun fetch/unpack and therefore revert
adjust_git.

The problem should be gone with the adjust_git patches from me or Jan.
Or should be re-analyzed on top of those.

Henning

> Jan
>

Henning Schild

unread,
Aug 16, 2021, 7:36:07 AM8/16/21
to Jan Kiszka, isar-users
Am Sun, 15 Aug 2021 20:55:34 +0200
schrieb Jan Kiszka <jan.k...@siemens.com>:
Does that cover all three envs? (docker-host, docker, and chroots?)

If your ../../../.. thing works that might be fine, but this whole
approach with "sed" is git surgery not backed by documentation or
official interfaces.
The benefit of the deduplication is likely neglectible ... unless you
take the kernel as example. But even there you are probably saving much
more with going shallow than with deduplication via shared clones.

Henning

> Jan
>

Uladzimir Bely

unread,
Aug 16, 2021, 8:49:35 AM8/16/21
to isar-users, Jan Kiszka
One of the easiest way, for example:

bitbake -v mc:qemuamd64-buster:cowsay

edit conf/local.conf # (e.g, (un)comment something global, like
ISAR_USE_CACHED_BASE_REPO)

bitbake -v mc:qemuamd64-buster:cowsay

Jan Kiszka

unread,
Aug 16, 2021, 12:57:37 PM8/16/21
to Henning Schild, isar-users
You are welcome to test. I've completed all tests successfully so far.

Note that it does not resolve the kas-container specific issue (nor does
your patch) that the download dir is generally different inside and
outside of the container. But kas-container shell is happy now.

> If your ../../../.. thing works that might be fine, but this whole
> approach with "sed" is git surgery not backed by documentation or
> official interfaces.
> The benefit of the deduplication is likely neglectible ... unless you
> take the kernel as example. But even there you are probably saving much
> more with going shallow than with deduplication via shared clones.
>

Given the significant drawbacks of deduplications and - so far - no
known remaining limitations of the relative alternate path, there is no
reason for add this patch.

Also note that your patch leaves the mirror link issue unresolved. That
sails under "adjust_git" but is not about patching alternates.

Therefore: NACK for this path.

Uladzimir Bely

unread,
Aug 19, 2021, 2:17:45 AM8/19/21
to isar-users, Jan Kiszka
In the email from понедельник, 16 августа 2021 г. 13:45:12 +03 user Jan Kiszka
I finally found the reason of failing patch in ISAR, after debugging and
comparing the oe/patch.py behaviour in ISAR and OE.

At the end, the problem of meta-isar/recipes-app/cowsay/files/isar.patch is
that it's not in git format. So, when GitApplyTree class tries to apply it, it
fails and switches to fallback PatchTree one.

That's why the patch can be applied only once - PatchTree class can't apply
the same patch if it has already been applied at previous build.

When I modify isar.patch to be git-like ( recreate it with `git format-
patch`), everythong works OK. GitApplyTree is now able to understand that the
same patch need to be skipped.

The difference in output:

> ubely@home /home/Work/isar/build-isar/tmp/work/debian-buster-amd64/cowsay/
git-r0/git $ PATCHFILE="isar.patch" git --work-tree=/home/Work/isar/build-
isar/tmp/work/debian-buster-amd64/cowsay/git-r0/git -c user.name="Isar" -c
user.email="isar.patch@isar" am -3 --keep-cr -p1 /home/Work/isar/repo/meta-
isar/recipes-app/cowsay/files/isar.patch
> Patch format detection failed.

> ubely@home /home/Work/isar/build-isar/tmp/work/debian-buster-amd64/cowsay/
git-r0/git $ PATCHFILE="isar.patch" git --work-tree=/home/Work/isar/build-
isar/tmp/work/debian-buster-amd64/cowsay/git-r0/git -c user.name="Isar" -c
user.email="isar.patch@isar" am -3 --keep-cr -p1 /home/Work/isar/repo/meta-
isar/recipes-app/cowsay/files/0001-original-patch-isar.patch.patch
> Applying: %% original patch: isar.patch
> Using index info to reconstruct a base tree...
> M debian/patches/series
> Falling back to patching base and 3-way merge...
> No changes -- Patch already applied.

So, I guess, we need to reformat isar.patch to be sure it can properly work
when PATCHTOOL="git"

Jan Kiszka

unread,
Aug 19, 2021, 2:21:37 AM8/19/21
to Uladzimir Bely, isar-users
Ah, great, thanks for debugging! Please send a patch - for the patch.

Jan Kiszka

unread,
Aug 26, 2021, 3:57:26 AM8/26/21
to Uladzimir Bely, isar-users
Did you send this patch already?

Jan Kiszka

unread,
Aug 26, 2021, 4:00:27 AM8/26/21
to isar-users, Uladzimir Bely, Henning Schild
From: Jan Kiszka <jan.k...@siemens.com>

This is needed since e47ada143469 because we now adjust the links to
mirrors in DL_DIR/git which may collide with other tasks doing this at
the same time.

Locking will also be needed soon for establishing the host-side link
to the download dir.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

Changes in v3:
- move lock into git folder
- fix commit message

meta/classes/dpkg-base.bbclass | 1 +
1 file changed, 1 insertion(+)

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index ec8fbc1..2cff1b3 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -55,6 +55,7 @@ python do_adjust_git() {
}

addtask adjust_git before do_dpkg_build
+do_adjust_git[lockfiles] += "${DL_DIR}/git/isar.lock"

inherit patch
addtask patch before do_adjust_git
--
2.31.1

Jan Kiszka

unread,
Aug 26, 2021, 4:06:11 AM8/26/21
to isar-users, Uladzimir Bely, Henning Schild
Hmpf, causes collisions further downstream, will resend whole series.

Jan Kiszka

unread,
Aug 26, 2021, 5:23:01 AM8/26/21
to isar-users, Uladzimir Bely, Henning Schild
From: Jan Kiszka <jan.k...@siemens.com>

This is needed since e47ada143469 because we now adjust the links to
mirrors in DL_DIR/git which may collide with other tasks doing this at
the same time.

Locking will also be needed soon for establishing the host-side link
to the download dir.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---
meta/classes/dpkg-base.bbclass | 1 +
1 file changed, 1 insertion(+)

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index ec8fbc1..2cff1b3 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -55,6 +55,7 @@ python do_adjust_git() {
}

addtask adjust_git before do_dpkg_build
+do_adjust_git[lockfiles] += "${DL_DIR}/git/isar.lock"

inherit patch
addtask patch before do_adjust_git
--
2.31.1

Jan Kiszka

unread,
Aug 26, 2021, 5:23:01 AM8/26/21
to isar-users, Uladzimir Bely, Henning Schild
Finally allow the adjusted git repos work both inside and outside the
buildchroots. Also fixes the race introduced by patching links in
DL_DIR.

Changes in v3:
- use /git for lock
- fix typos in commit logs

Changes in v2:
- build relative alternates path so that destsuffix paths are respected
- fix task dependencies so that devshell gets adjusted git again

Jan

Jan Kiszka (4):
dpkg-base: Lock do_adjust_git against each other
dpkg-base: Make mirror link relative
Rework do_adjust_git to support inside and outside usage
Revert "dpkg: adjust task order to allow using "git" for patching"

meta/classes/dpkg-base.bbclass | 22 ++++++++++++++-----
meta/conf/bitbake.conf | 1 +
.../buildchroot/buildchroot.inc | 2 ++
3 files changed, 20 insertions(+), 5 deletions(-)

--
2.31.1

Jan Kiszka

unread,
Aug 26, 2021, 5:23:02 AM8/26/21
to isar-users, Uladzimir Bely, Henning Schild
From: Jan Kiszka <jan.k...@siemens.com>

This reverts commit b4c33ba69ed89849ecf32d8731aa97ecf88226f5.

No longer needed now that adjust_git stopped breaking the outer
environment. Moreover, this was broken /wrt devshell anyway.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---
meta/classes/dpkg-base.bbclass | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index 9a18acb..8286168 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -65,11 +65,11 @@ python do_adjust_git() {
bb.fatal(str(e))
}

-addtask adjust_git before do_dpkg_build
+addtask adjust_git after do_unpack before do_patch
do_adjust_git[lockfiles] += "${DL_DIR}/git/isar.lock"

inherit patch
-addtask patch before do_adjust_git
+addtask patch after do_adjust_git before do_dpkg_build

SRC_APT ?= ""

--
2.31.1

Jan Kiszka

unread,
Aug 26, 2021, 5:23:02 AM8/26/21
to isar-users, Uladzimir Bely, Henning Schild
From: Jan Kiszka <jan.k...@siemens.com>

This will allow to use it both from the outer builder context and inside
the buildchroot. No idea why bitbake doesn't do that in the first place.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---
meta/classes/dpkg-base.bbclass | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index 2cff1b3..ca1dccf 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -28,8 +28,10 @@ python do_adjust_git() {

if os.path.islink(ud.localpath):
realpath = os.path.realpath(ud.localpath)
- if realpath.startswith(d.getVar("DL_DIR")):
- link = realpath.replace(d.getVar("DL_DIR"), '/downloads', 1)
+ filter_out = os.path.join(d.getVar("DL_DIR"), "git") + "/"
+ if realpath.startswith(filter_out):
+ # make the link relative
+ link = realpath.replace(filter_out, '', 1)
os.unlink(ud.localpath)
os.symlink(link, ud.localpath)

--
2.31.1

Jan Kiszka

unread,
Aug 26, 2021, 5:23:02 AM8/26/21
to isar-users, Uladzimir Bely, Henning Schild
From: Jan Kiszka <jan.k...@siemens.com>

Rewrite the alternates entry to use a relative path outside of the
workdir. That linke can then be created independently of the outer build
environment as well as inside the buildchroot.

We use ${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/.git-downloads outside
and /home/.git-downloads inside the buildchroot.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---
meta/classes/dpkg-base.bbclass | 11 ++++++++++-
meta/conf/bitbake.conf | 1 +
meta/recipes-devtools/buildchroot/buildchroot.inc | 2 ++
3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index ca1dccf..9a18acb 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -19,6 +19,12 @@ python do_adjust_git() {

rootdir = d.getVar('WORKDIR', True)

+ git_link = os.path.join(d.getVar('GIT_DL_LINK_DIR'), '.git-downloads')
+ git_dl = os.path.join(d.getVar("DL_DIR"), "git")
+
+ if not os.path.exists(git_link) or os.path.realpath(git_link) != git_dl:
+ os.symlink(git_dl, git_link)
+
for src_uri in (d.getVar("SRC_URI", True) or "").split():
try:
fetcher = bb.fetch2.Fetch([src_uri], d)
@@ -44,11 +50,14 @@ python do_adjust_git() {
destsuffix = ud.parm.get("destsuffix", def_destsuffix)
destdir = ud.destdir = os.path.join(rootdir, destsuffix)

+ git_link_rel = os.path.relpath(git_link,
+ os.path.join(destdir, ".git/objects"))
+
alternates = os.path.join(destdir, ".git/objects/info/alternates")

if os.path.exists(alternates):
cmd = ["sed", "-i", alternates, "-e",
- "s|{}|/downloads|".format(d.getVar("DL_DIR"))]
+ "s|{}|{}|".format(git_dl, git_link_rel)]
bb.note(' '.join(cmd))
if subprocess.call(cmd) != 0:
bb.fatal("git alternates adjustment failed")
diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
index 9859456..7f5901d 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -48,6 +48,7 @@ GITPKGV = "${@bb.fetch2.get_srcrev(d, 'gitpkgv_revision')}"

# isar specific config
WORKDIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/${PN}/${PV}-${PR}"
+GIT_DL_LINK_DIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}"
DEPLOY_DIR_BOOTSTRAP = "${DEPLOY_DIR}/bootstrap"
DEPLOY_DIR_BUILDCHROOT = "${DEPLOY_DIR}/buildchroot"
DEPLOY_DIR_SDKCHROOT = "${DEPLOY_DIR}/sdkchroot"
diff --git a/meta/recipes-devtools/buildchroot/buildchroot.inc b/meta/recipes-devtools/buildchroot/buildchroot.inc
index 0c1fb7f..52a5f1c 100644
--- a/meta/recipes-devtools/buildchroot/buildchroot.inc
+++ b/meta/recipes-devtools/buildchroot/buildchroot.inc
@@ -67,6 +67,8 @@ buildchroot_install_files() {
sudo install -m 755 ${WORKDIR}/common.sh ${BUILDCHROOT_DIR}/isar/
sudo install -m 755 ${WORKDIR}/deps.sh ${BUILDCHROOT_DIR}/isar/

+ sudo ln -s /downloads/git "${BUILDCHROOT_DIR}/home/.git-downloads"
+
# Configure root filesystem
sudo install -m 755 ${WORKDIR}/configscript.sh ${BUILDCHROOT_DIR}
USER_ID=$(id -u)
--
2.31.1

ub...@ilbers.de

unread,
Aug 27, 2021, 4:39:55 AM8/27/21
to isar-users
I've just sent a patchset as "[RFC 0/3] Fix gbp patching after configuration change" (sorry, with wrong tag RFC) after internal discussion.
четверг, 26 августа 2021 г. в 10:57:26 UTC+3, Jan Kiszka:

Anton Mikanovich

unread,
Sep 9, 2021, 6:42:56 AM9/9/21
to Jan Kiszka, isar-users, Uladzimir Bely, Henning Schild
Applied to next, thanks.

--
Anton Mikanovich
Reply all
Reply to author
Forward
0 new messages