Re: [PATCH] dpkg-base: "unshare" git clones in adjust_git

[Jan Kiszka <jan.kiszka@siemens.com>]

On 16.08.21 13:36, Henning Schild wrote:
> Am Sun, 15 Aug 2021 20:55:34 +0200
> schrieb Jan Kiszka <jan.kiszka@siemens.com>:
> 
>> On 13.08.21 19:49, Henning Schild wrote:
>>> 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.schild@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 ?= ""
>>>  
>>>   
>>
>> NACK, we have a pattern now that avoids this.
> 
> Does that cover all three envs? (docker-host, docker, and chroots?)
> 

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.

Jan

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