Re: [PATCH v5 0/5] Debootstrap integration

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

Hi all,

On 2018-04-05 10:03, [ext] Claudius Heine wrote:
> Hi Baurzhan,
> 
> On 04/04/2018 10:34 PM, Baurzhan Ismagulov wrote:
>> On Tue, Apr 03, 2018 at 12:07:57PM +0200,
>> claudius.heine.ext@siemens.com wrote:
>>> this is the new version of this patchset, that fixes the
>>> generate_keyring task in isar-bootstrap for systems with read-only
>>> homedir.
>>
>> Thanks, worked fine on my host. CI still in progress.
>>
>>
>> It's unfortunate that the series introduces regressions you wrote about
>> (changing mirrors, setting hostname). It's always better to fix the
>> issues on
>> the spot. If there are no objections, I'd like to add TODOs to the
>> patches.
>> Please let me know whether it's ok, or you would like to address those
>> before
>> the merge.
> 
> I don't know about you, but I prefer having TODOs somewhere outside the
> project, maybe in the github issue tracker. At least in my experience
> TODOs together with code or in a separate file inside the repo are
> seldom updated and easily forgotten. If this project prefers having
> TODOs inside the repo, then sure, I have nothing against adding them
> somewhere.
> 
> I do plan of adding more features to this once this is merged. This
> patchset just provides the baseline.
> 
>> What I'd really like to see is an update to doc/user_manual.md. Would
>> you have
>> time for that in the next days?
> 
> I'll try.
> 
>>
>>
>> If I understand the code correctly, there is also a security issue:
> 
> Not sure if security is really a concern for isar yet. But I get your
> point that we should prevent possible accidents. :)
> 
>>
>> On Tue, Apr 03, 2018 at 12:08:00PM +0200,
>> claudius.heine.ext@siemens.com wrote:
>>> +   
>>> CDIRS="${@d.expand(d.getVarFlags("do_build").get("root_cleandirs",
>>> ""))}"
>>> +    if [ -n "$CDIRS" ]; then
>>> +        sudo rm -rf $CDIRS
>>> +        mkdir -p $CDIRS
>>> +    fi
>>
>> Should root_cleandirs items be checked for directory traversal ("/",
>> "..") and
>> mounted filesystems in the subdirectories? If yes, do we want to drop the
>> feature from this series and address the issue in a separate step?
> 
> This isn't really a new feature of isar yet. Its just the start of a
> general interface, that could be use everywhere when its acknowledged by
> the community and fully implemented. So it has to be improved anyway.
> 
> So I would say its good enough in this case, since setting those
> directories in the flag and removing them is currently bundled together
> in the same file. If we later centralized this step somewhere
> (base.bbclass) to make it available for every task, then checking it
> more thoroughly has to be done there.
> So maybe add centralization of the 'root_cleandirs' task flag to the
> TODO list as well.
> 
> Cheers,
> Claudius
> 

what's the status of this series from upstream perspective now? Claudius
sent a documentation update. Any further requirements that need to be
fulfilled prior to this becoming ready for merge?

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux