Re: [PATCH v5 07/12] sbuild: support of shell exports from dpkg_runbuild_prepend

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

On 01.02.22 19:41, Uladzimir Bely wrote:
> In the email from Tuesday, 1 February 2022 21:09:07 +03 user Jan Kiszka wrote:
>> On 01.02.22 18:00, Uladzimir Bely wrote:
>>> Many of recipes often use shell exports done in dpkg_run_prepend, so that
>>> this changed environment is used during build.
>>>
>>> While sbuild is performed in isolated environment, we need a way to
>>> pass these variables to it. This is done by storing environment before
>>> dpkg_runbuild (after prepare_build) and finding just before the actual
>>> build what was changed or added.
>>>
>>> Signed-off-by: Uladzimir Bely <ubely@ilbers.de>
>>> ---
>>>
>>>  meta/classes/dpkg.bbclass | 18 ++++++++++++++++++
>>>  1 file changed, 18 insertions(+)
>>>
>>> diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
>>> index 66db7ec5..c252e9b3 100644
>>> --- a/meta/classes/dpkg.bbclass
>>> +++ b/meta/classes/dpkg.bbclass
>>> @@ -29,12 +29,30 @@ do_install_builddeps[lockfiles] +=
>>> "${REPO_ISAR_DIR}/isar.lock"> 
>>>  addtask devshell after do_install_builddeps
>>>
>>> +DPKG_PREBUILD_ENV_FILE="${WORKDIR}/dpkg_prebuild.env"
>>> +
>>> +do_prepare_build_append() {
>>> +    env > ${DPKG_PREBUILD_ENV_FILE}
>>> +}
>>> +
>>>
>>>  # Build package from sources using build script
>>>  dpkg_runbuild() {
>>>  
>>>      E="${@ isar_export_proxies(d)}"
>>>      E="${@ isar_export_ccache(d)}"
>>>      export PARALLEL_MAKE="${PARALLEL_MAKE}"
>>>
>>> +    env | while read -r line; do
>>> +        # Filter the same lines
>>> +        grep -q "^${line}" ${DPKG_PREBUILD_ENV_FILE} && continue
>>> +        # Filter some standard variables
>>> +        echo ${line} | grep -q "^HOME=" && continue
>>> +        echo ${line} | grep -q "^PWD=" && continue
>>> +
>>> +        var=$(echo "${line}" | cut -d '=' -f1)
>>> +        value=$(echo "${line}" | cut -d '=' -f2-)
>>> +        sbuild_export $var "$value"
>>> +    done
>>> +
>>>
>>>      profiles=$(grep "DEB_BUILD_PROFILES" ${SBUILD_CONFIG} | tail -n1 |
>>>      cut -d "'" -f 4) if [ ${ISAR_CROSS_COMPILE} -eq 1 ]; then
>>>      
>>>          profiles="${profiles} cross nocheck"
>>
>> So, this basically decouples "Avoid using shell environment during the
>> build" and similar conversions downstream from this series, right? It's
>> indeed good to have a compat path now.
>>
>> However, what are the patterns we want to push? Avoiding exports? Then
>> we should probably warn here that this compat path should be avoided and
>> might be removed in the future.
>>
>> If we want to keep both, we can probably leave several recipes alone in
>> that other series.
>>
>> Jan
> 
> You might be right. Current solution doesn't force users to avoid using 
> exports while they are silently supported.
> 
> The first internal solution was keeping dpkg_build_export function but also 
> passing custom shell exports (from dpkg_runbuild_prepend) to sbuild 
> environment. And a warning about migration was shown.
> 
> Later I removed this to make new patchset less invasive for downstreams. 
> Anyway, if we agree that "dpkg_build_export" (like API function) is more 
> preferable, I can get back this solution with warnings.
> 

This is not necessarily what I'm suggesting. Your conversion series
works without that API, primarily using template. We also have build
profiles. But we first of all need to define what we actually want.

The benefit of pushing things into the build env, rather than feeding it
from the recipe, is likely that the resulting deb-src of such generated
packages can actually be used for rebuilding independently of Isar. So
there is likely value in deprecating classic export.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux