~qaul/community

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
3 2

[PATCH 1/2] clients/android: don't override USER in build.sh

Alyssa Ross
Details
Message ID
<20200531181702.6226-1-hi@alyssa.is>
DKIM signature
pass
Download raw message
Patch: +6 -6
USER is a standard global environment variable that contains the name
of the current user.  Previously, build.sh would reassign user to be
the id of the current user.  This would break podman, which uses $USER
to find the name of the user to look up in /etc/subuid.

Since environment variables are conventionally upper case, using
lowercase variable names in scripts avoids the problem of overriding
variables in the environment.
---
 clients/android/build.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/clients/android/build.sh b/clients/android/build.sh
index 44cba421..eb559798 100755
--- a/clients/android/build.sh
+++ b/clients/android/build.sh
@@ -2,18 +2,18 @@

set -e

BASEDIR=$(realpath $(dirname "$0"))
USER=$(id -u)
GROUP=$(id -g)
basedir=$(realpath $(dirname "$0"))
user=$(id -u)
group=$(id -g)

if [ $1 = "dev" ]; then
    echo "Attaching shell for repeated builds."
    echo "Don't invoke gradle yourself! Use 'client/android/.build_nested.sh' instead!"
    echo "Don't forget to run 'export USER=$USER GROUP=$GROUP'!"
    docker run --rm -it -v $BASEDIR/../../:/qaul.net qaulnet/android-build-env /bin/bash
    docker run --rm -it -v $basedir/../../:/qaul.net qaulnet/android-build-env /bin/bash
  
else
    echo "Running one-shot-build"
    docker run --rm -it -v $BASEDIR/../../:/qaul.net qaulnet/android-build-env \
           /qaul.net/clients/android/.build_nested.sh $USER $GROUP
    docker run --rm -it -v $basedir/../../:/qaul.net qaulnet/android-build-env \
           /qaul.net/clients/android/.build_nested.sh $user $group
fi
-- 
2.26.2

[PATCH 2/2] clients/android: fix build.sh undefined variable

Alyssa Ross
Details
Message ID
<20200531181702.6226-2-hi@alyssa.is>
In-Reply-To
<20200531181702.6226-1-hi@alyssa.is> (view parent)
DKIM signature
pass
Download raw message
Patch: +1 -1
If no argument was given, this script would produce the error:

    ./build.sh: line 9: [: =: unary operator expected

This happened because the expansion of the [ command became

    [ = "dev" ]

Adding the quotes around $1 makes the expansion

    [ "" = "dev" ]

Which is what we actually want.
---
 clients/android/build.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clients/android/build.sh b/clients/android/build.sh
index eb559798..b644bef0 100755
--- a/clients/android/build.sh
+++ b/clients/android/build.sh
@@ -6,7 +6,7 @@ basedir=$(realpath $(dirname "$0"))
user=$(id -u)
group=$(id -g)

if [ $1 = "dev" ]; then
if [ "$1" = "dev" ]; then
    echo "Attaching shell for repeated builds."
    echo "Don't invoke gradle yourself! Use 'client/android/.build_nested.sh' instead!"
    echo "Don't forget to run 'export USER=$USER GROUP=$GROUP'!"
-- 
2.26.2
Alyssa Ross
Details
Message ID
<87v9kba6yx.fsf@alyssa.is>
In-Reply-To
<20200531181702.6226-1-hi@alyssa.is> (view parent)
DKIM signature
pass
Download raw message
> USER is a standard global environment variable that contains the name
> of the current user.  Previously, build.sh would reassign user to be
> the id of the current user.  This would break podman, which uses $USER
> to find the name of the user to look up in /etc/subuid.
>
> Since environment variables are conventionally upper case, using
> lowercase variable names in scripts avoids the problem of overriding
> variables in the environment.
> [snip]
> 
>  if [ $1 = "dev" ]; then
>      echo "Attaching shell for repeated builds."
>      echo "Don't invoke gradle yourself! Use 'client/android/.build_nested.sh' instead!"
>      echo "Don't forget to run 'export USER=$USER GROUP=$GROUP'!"

This should have been changed too, but I'm not sure what these
instructions are actually for.  I think the idea is that
.build_nested.sh uses them, but looking at the source for that, it
actually uses positional arguments instead.

So I'm not really sure what to do with this until that's clarified.
Details
Message ID
<87lfkjz66u.fsf@kookie.space>
In-Reply-To
<87v9kba6yx.fsf@alyssa.is> (view parent)
DKIM signature
missing
Download raw message
Hey there,

>>  if [ $1 = "dev" ]; then
>>      echo "Attaching shell for repeated builds."
>>      echo "Don't invoke gradle yourself! Use 'client/android/.build_nested.sh' instead!"
>>      echo "Don't forget to run 'export USER=$USER GROUP=$GROUP'!"
>
> This should have been changed too, but I'm not sure what these
> instructions are actually for.  I think the idea is that
> .build_nested.sh uses them, but looking at the source for that, it
> actually uses positional arguments instead.
>
> So I'm not really sure what to do with this until that's clarified.

Ah yes, that's kinda wrong at the moment.

The ./build_nested.sh script shouldn't use positional paraments because
it makes using it inside the container shell much more annoying.  So
maybe we just wanna set QAUL_USER and QAUL_GROUP as env vars then that
the script can use? (don't actually know how we spawn a shell in the
container that has a certain env set in case of the dev setup.

Anyway, long story short: remove the 'export ...' instructions, and make
both build.sh and .build_nested.sh use a custom env var to store the
UIDs.  Does that make sense?
Reply to thread Export thread (mbox)