Re: [PATCH v8 1/7] Revert "isar-bootstrap: Allow to set local keys in DISTRO_APT_KEYS"

["Maxim Yu. Osipov" <mosipov@ilbers.de>]

On 4/16/19 10:12 AM, Henning Schild wrote:
> Am Tue, 16 Apr 2019 06:54:51 +0200
> schrieb "Maxim Yu. Osipov" <mosipov@ilbers.de>:
> 
>> On 4/15/19 1:11 PM, Andreas Reichel wrote:
>>> On Mon, Mar 25, 2019 at 12:20:10PM +0100, Maxim Yu. Osipov wrote:
>>>> Hi Andreas,
>>>>
>>>> On 3/21/19 4:15 PM, Andreas J. Reichel wrote:
>>>>> From: Andreas Reichel <andreas.reichel.ext@siemens.com>
>>>>>
>>>>> This reverts commit af983a13b6f4cee5d4af5e5cf6318231e02775c9.
>>>>>
>>>>> This commit broke usage of remote keys, where they usually come
>>>>> from. If fetching "http://example.com/dir1/dir2/key", the file is
>>>>> fetched into the subdir WORKDIR/dir1/dir2/, which breaks with
>>>>> this code. However it succeeds with absolute paths.
>>>>> We do not want to guess where the downloaded file will be. This
>>>>> does not work anymore if the key is downloaded from remote with a
>>>>> URL. Furthermore, a user could specify "subdir" as fetcher
>>>>> parameter or other things, which break this.
>>>>
>>>> In general it's better to avoid reverting the patches.
>>>
>>> Okay.
>>>    
>>>>
>>>> I disagree with the statement that "commit af983a13  broke usage
>>>> of remote keys" - I wrote you a month ago (see forwarded email
>>>> below) that this patch doesn't break Raspbian build  - the key is
>>>> put also under downloads (DL_DIR)
>>>
>>> in DL_DIR - Yes, where you can get name collisions with several
>>> keys since you cannot rename the files with your code via bitbake
>>> URL parameter.
>>
>> Well...And how this problem is related with my patch?
>> My patch fixes the problem with locally stored keys, as
>> Local fetcher module puts the file under
>> ${WORKDIR}/<absolute_path_to_key_file>, so
>> gpg in do_generate_keyring() task can't find it.
>>
>>
>>>> directory:
>>>>
>>>> I've build the current 'master' for target
>>>> multiconfig:rpi-jessie:isar-image-base.
>>>
>>> I gave you an example where it broke for me. Raspbian is not one of
>>> these.
>>>
>>>    https://archive.raspbian.org/raspbian.public.key
>>>
>>>    Is there a subdir after the domain? - No ! Does it fit to my
>>> example? => No. Does your argument with raspbian make sense now? =>
>>> No.
>>
>> Again, my patch fixes the problem with local keys.
>> It does nothing about fetching remote keys - it just doesn't break
>> existing functionality.
> 
> While i do agree that a revert looks like blaming a patch and an author
> for doing something wrong, i fail to get why this comes up so late in
> the review. 

My first feedback was ignored by unknown reason.

Yes Andreas can rebase yet again and turn the revert into
> something else, in the end the code will be the same.

> 
> The revert question was already discussed in v3. Let us talk content
> and that series is in very good shape.
> 
> I would suggest to merge that and accept a test-case / example on top.

Not merged, as problems with the latest v9 series were discovered during 
testing.

Maxim.

> 
> Henning
> 
>>
>>>>
>>>> Sorry for copy-paste ;):
>>>>
>>>> <<<<
>>>> isar/build$ find -name raspbian.public.key
>>>> ./downloads/raspbian.public.key
>>>> ./tmp/work/raspbian-jessie-armhf/isar-bootstrap-target/raspbian.public.key
>>>>>>>   
>>>>
>>>> So I don't accept this patch.
>>>>   
>>> Then I have to rebase - without revert - , which may take some
>>> time...
>>
>> OK.
>>
>> Regards,
>> Maxim.
>>
>>> Regards,
>>> Andreas
>>>    
>>>> Regards,
>>>> Maxim.
>>>>
>>>>
>>>> -------- Forwarded Message --------
>>>> Subject: Re: [PATCH 0/1] Fix remote key fetching apt keyring
>>>> Date: Wed, 20 Feb 2019 12:58:09 +0100
>>>> From: Maxim Yu. Osipov <mosipov@ilbers.de>
>>>> Organization: ilbers GmbH
>>>> To: Jan Kiszka <jan.kiszka@siemens.com>, [ext] Andreas J. Reichel
>>>> <andreas.reichel.ext@siemens.com>, isar-users@googlegroups.com,
>>>> Baurzhan Ismagulov <ibr@ilbers.de>
>>>>
>>>> On 2/20/19 12:27 PM, Jan Kiszka wrote:
>>>>> On 20.02.19 12:21, [ext] Andreas J. Reichel wrote:
>>>>>> From: Andreas Reichel <andreas.reichel.ext@siemens.com>
>>>>>>
>>>>>> Since my last mail was not answered, but this is an important
>>>>>> topic, here is a patch that shows what the problem is.
>>>>>>
>>>>>> If we fetch the user apt key from remote, we need the basename,
>>>>>> if we fetch it locally we need the absolute path...
>>>>>>
>>>>>> While this might not be the best way to fix this, it works as
>>>>>> good as the rest of this code...
>>>>>>
>>>>>> At least it fixes Isar again up to adding the key to the keyring.
>>>>>>
>>>>>> But this still does not fix the next problem with the docker-ce
>>>>>> key:
>>>>>>
>>>>>> | I: Running command: debootstrap --arch arm64 --foreign
>>>>>> --verbose --variant=minbase --include=locales
>>>>>> --components=main,contrib,non-free --keyring
>>>>>> /build/build/tmp/work/debian-stretch-arm64/isar-bootstrap-target/apt-keyring.gpg
>>>>   
>>>>>> stretch
>>>>>> /build/build/tmp/work/debian-stretch-arm64/isar-bootstrap-target/rootfs
>>>> http://ftp.debian.org/debian
>>>>>>
>>>>>> | I: Retrieving InRelease
>>>>>> | I: Retrieving Release
>>>>>> | I: Retrieving Release.gpg
>>>>>> | I: Checking Release signature
>>>>>> | E: Release signed by unknown key (key id EF0F382A1A7B6500)
>>>>>>
>>>>>> So something additionally must be done. Since I am not an expert
>>>>>> on debian keyring/debootstrap and dpkg signing I will try to
>>>>>> find a solution but maybe somebody has a good idea already?
>>>>>>   
>>>>>
>>>>> Baurzhan, Maxim, any idea?
>>>>
>>>> Strange...I thought that commit af983a13 fixes the reported problem
>>>> When testing my patch signing base-apt I've tried both - remote
>>>> keys (used by Raspberry Pi target) and local key.
>>>>
>>>>
>>>>   
>>>>>
>>>>> This is really fixed in a follow-up commit.
>>>>>
>>>>> Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com>
>>>>> ---
>>>>>     meta/recipes-core/isar-bootstrap/isar-bootstrap.inc | 5 +++--
>>>>>     1 file changed, 3 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
>>>>> b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc index
>>>>> c1b571a..2910eea 100644 ---
>>>>> a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc +++
>>>>> b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc @@ -40,8
>>>>> +40,9 @@ python () { d.setVar("DEBOOTSTRAP_KEYRING", "--keyring
>>>>> ${APTKEYRING}") for key in distro_apt_keys.split():
>>>>>                 url = urlparse(key)
>>>>> -            d.appendVar("SRC_URI", " " + key)
>>>>> -            d.appendVar("APTKEYFILES", " " + wd + url.path)
>>>>> +            filename = os.path.basename(url.path)
>>>>> +            d.appendVar("SRC_URI", " %s" % key)
>>>>> +            d.appendVar("APTKEYFILES", " %s" % filename)
>>>>>         if
>>>>> bb.utils.to_boolean(d.getVar('ISAR_USE_CACHED_BASE_REPO')):
>>>>> own_pub_key = d.getVar("BASE_REPO_KEY", False) if own_pub_key:
>>>>>   
>>>>
>>>>
>>>> -- 
>>>> Maxim Osipov
>>>> ilbers GmbH
>>>> Maria-Merian-Str. 8
>>>> 85521 Ottobrunn
>>>> Germany
>>>> +49 (151) 6517 6917
>>>> mosipov@ilbers.de
>>>> http://ilbers.de/
>>>> Commercial register Munich, HRB 214197
>>>> General Manager: Baurzhan Ismagulov
>>>    
>>
>>
> 


-- 
Maxim Osipov
ilbers GmbH
Maria-Merian-Str. 8
85521 Ottobrunn
Germany
+49 (151) 6517 6917
mosipov@ilbers.de
http://ilbers.de/
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov