[Mesa-dev] [PATCH] gitlab-ci: remove software-properties-common

[Michel Dänzer]

On 2019-08-01 2:32 p.m., Emil Velikov wrote:
> On Thu, 1 Aug 2019 at 09:35, Michel Dänzer <michel at daenzer.net> wrote:
>>
>> On 2019-07-31 11:47 p.m., Eric Engestrom wrote:
>>> On Wednesday, 2019-07-31 16:07:45 +0200, Michel Dänzer wrote:
>>>> On 2019-07-31 3:26 p.m., Emil Velikov wrote:
>>>>> On Wed, 31 Jul 2019 at 14:16, Michel Dänzer <michel at daenzer.net> wrote:
>>>>>>
>>>>>> On 2019-07-31 3:04 p.m., Emil Velikov wrote:
>>>>>>> From: Emil Velikov <emil.velikov at collabora.com>
>>>>>>>
>>>>>>> Currently we use the python package to manage repositories. At the same
>>>>>>> time we also do that by hand - since it's a trivial echo to a file.
>>>>>>>
>>>>>>> Stay consistent, remove the package and manage things manually.
>>>>>>>
>>>>>>> Cc: Eric Engestrom <eric.engestrom at intel.com>
>>>>>>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>>>>>>> ---
>>>>>>>  .gitlab-ci/debian-install.sh | 11 +++++------
>>>>>>>  1 file changed, 5 insertions(+), 6 deletions(-)
>>>>>>>
>>>>>>> diff --git a/.gitlab-ci/debian-install.sh b/.gitlab-ci/debian-install.sh
>>>>>>> index 578074ddb87..719d7830018 100644
>>>>>>> --- a/.gitlab-ci/debian-install.sh
>>>>>>> +++ b/.gitlab-ci/debian-install.sh
>>>>>>> @@ -16,12 +16,11 @@ apt-get install -y \
>>>>>>>        curl \
>>>>>>>        wget \
>>>>>>>        unzip \
>>>>>>> -      gnupg \
>>>>>>> -      software-properties-common
>>>>>>> +      gnupg
>>>>>>>
>>>>>>>  curl -fsSL https://apt.llvm.org/llvm-snapshot.gpg.key | apt-key add -
>>>>>>> -add-apt-repository "deb https://apt.llvm.org/stretch/ llvm-toolchain-stretch-7 main"
>>>>>>> -add-apt-repository "deb https://apt.llvm.org/stretch/ llvm-toolchain-stretch-8 main"
>>>>>>> +echo "deb [trusted=yes] https://apt.llvm.org/stretch/ llvm-toolchain-stretch-7 main" >/etc/apt/sources.list.d/llvm7.list
>>>>>>> +echo "deb [trusted=yes] https://apt.llvm.org/stretch/ llvm-toolchain-stretch-8 main" >/etc/apt/sources.list.d/llvm8.list
>>>>>>>
>>>>>>>  sed -i -e 's/http:\/\/deb/https:\/\/deb/g' /etc/apt/sources.list
>>>>>>>  echo 'deb https://deb.debian.org/debian stretch-backports main' >/etc/apt/sources.list.d/backports.list
>>>>>>> @@ -46,8 +45,8 @@ apt-get install -y -t stretch-backports \
>>>>>>>        clang-8
>>>>>>>
>>>>>>>  # Install remaining packages from Debian buster to get newer versions
>>>>>>> -add-apt-repository "deb https://deb.debian.org/debian/ buster main"
>>>>>>> -add-apt-repository "deb https://deb.debian.org/debian/ buster-updates main"
>>>>>>> +echo "deb https://deb.debian.org/debian/ buster main" >/etc/apt/sources.list.d/buster.list
>>>>>>> +echo "deb https://deb.debian.org/debian/ buster-updates main" >/etc/apt/sources.list.d/buster-updates.list
>>>>>>>  apt-get update
>>>>>>>  apt-get install -y \
>>>>>>>        bzip2 \
>>>>>>>
>>>>>>
>>>>>> This should be merged as part of an MR which requires the docker image
>>>>>> to be re-generated for another reason, and thus bumps DEBIAN_TAG.
>>>>>>
>>>>> Since this is a non-functional change, I've explicitly omitted bumping
>>>>> the DEBIAN_TAG.
>>>>> Seemingly I forgot to mention it in the commit message though, oopsie.
>>>>>
>>>>> Since the image will contain practically the same artefacts, is it
>>>>> worth carving out 30 minutes (or so) from the runners?
>>>>
>>>> No, I agree that would be wasteful for this change alone.
>>>>
>>>> However, merging this change without bumping the tag isn't good either,
>>>> because then any issues with it would only be discovered the next time
>>>> it does get bumped. Hence my request above.
>>>
>>> I agree with Michel here, it's better to waste a re-gen now and notice
>>> any issue right away.
>>
>> That's not exactly what I'm saying though. :)
>>
>> If you don't want to merge this together with other changes which bump
>> the tag, how about:
>>
>> * Create an MR with an additional commit which bumps the tag
>> * Wait for the CI pipeline to come back green
>> * Force-push to the source branch without the additional commit, and
>>   merge the MR
>> * Remove the ephemeral docker image from
>> https://gitlab.freedesktop.org/evelikov/mesa/container_registry
>>
>> The CI pipeline information including the log of the job which generated
>> the ephemeral docker image will remain accessible.
>>
> if I understand you correctly, the goal is to have this explicitly tested.
> Since it's a no-op,

It's not a no-op, it has an observable effect, which is that the
software-properties-common package will no longer be installed in the
docker image. While it's unlikely that this will break anything, we have
the CI pipeline to make sure changes don't break anything, so let's use it.


> one might as well keep the test (and resulting extra image) downstream aka
> in my repo.

The image isn't needed after the test (that's why I call it ephemeral),
only a record that it was successfully generated and that the CI
pipeline's later stage jobs passed against it. An MR provides such a
record (on the "Pipelines" tab).


> Sure I'll do that in a moment.

Why don't you just follow my suggestion above?


-- 
Earthling Michel Dänzer               |              https://www.amd.com
Libre software enthusiast             |             Mesa and X developer