All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH-for-5.2 0/2] gitlab-ci: Fix EDK2 & OpenSBI jobs
@ 2020-11-10 12:16 Philippe Mathieu-Daudé
  2020-11-10 12:16 ` [PATCH-for-5.2 1/2] gitlab-ci: Avoid running the OpenSBI job when not necessary Philippe Mathieu-Daudé
  2020-11-10 12:16 ` [PATCH-for-5.2 2/2] gitlab-ci: Avoid running the EDK2 " Philippe Mathieu-Daudé
  0 siblings, 2 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-11-10 12:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Daniel P . Berrange, Laszlo Ersek,
	Philippe Mathieu-Daudé,
	Bin Meng, Alex Bennée

We had rules to restrict building the EDK2 & OpenSBI firmwares\r
on GitLab CI. Unfortunately commit 922febe2 ("Move edk2 and\r
opensbi YAML files to .gitlab-ci.d folder") made these rules\r
ineffective. Fix that. In particular avoid the EDK2 job burn\r
all free minutes for our fork users.\r
\r
Philippe Mathieu-Daudé (2):\r
  gitlab-ci: Avoid running the OpenSBI job when not necessary\r
  gitlab-ci: Avoid running the EDK2 job when not necessary\r
\r
 .gitlab-ci.d/edk2.yml    | 11 ++++-------\r
 .gitlab-ci.d/opensbi.yml | 13 -------------\r
 2 files changed, 4 insertions(+), 20 deletions(-)\r
\r
-- \r
2.26.2\r
\r



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH-for-5.2 1/2] gitlab-ci: Avoid running the OpenSBI job when not necessary
  2020-11-10 12:16 [PATCH-for-5.2 0/2] gitlab-ci: Fix EDK2 & OpenSBI jobs Philippe Mathieu-Daudé
@ 2020-11-10 12:16 ` Philippe Mathieu-Daudé
  2020-11-10 14:33   ` Daniel P. Berrangé
  2020-11-10 12:16 ` [PATCH-for-5.2 2/2] gitlab-ci: Avoid running the EDK2 " Philippe Mathieu-Daudé
  1 sibling, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-11-10 12:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Daniel P . Berrange, Laszlo Ersek,
	Philippe Mathieu-Daudé,
	Bin Meng, Alex Bennée

The OpenSBI jobs use the 'changes' keyword, which "makes it
possible to define if a job should be created based on files
modified by a Git push event." (see [*]). This keyword comes
with a warning:

  Caution:

    In pipelines with sources other than the three above
    changes can’t determine if a given file is new or old
    and always returns true."

In commit 922febe2af we moved the YAML config file from the
repository root directory to the .gitlab-ci.d/ directory.

We didn't respect the previous warning and disabled the
'changes' filter rule, as the files are now in a (directory)
three above the YAML config file.

As this job doesn't take much (less than 1min), run it by
default.

[*] https://docs.gitlab.com/ee/ci/yaml/#onlychangesexceptchanges

Fixes: 922febe2af ("Move edk2 and opensbi YAML files to .gitlab-ci.d folder")
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 .gitlab-ci.d/opensbi.yml | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/.gitlab-ci.d/opensbi.yml b/.gitlab-ci.d/opensbi.yml
index 5b13047e2ab..b1fa244fda4 100644
--- a/.gitlab-ci.d/opensbi.yml
+++ b/.gitlab-ci.d/opensbi.yml
@@ -1,10 +1,5 @@
 docker-opensbi:
  stage: containers
- rules: # Only run this job when the Dockerfile is modified
- - changes:
-   - .gitlab-ci.d/opensbi.yml
-   - .gitlab-ci.d/opensbi/Dockerfile
-   when: always
  image: docker:19.03.1
  services:
  - docker:19.03.1-dind
@@ -25,14 +20,6 @@ docker-opensbi:
 
 build-opensbi:
  stage: build
- rules: # Only run this job when ...
- - changes: # ... roms/opensbi/ is modified (submodule updated)
-   - roms/opensbi/*
-   when: always
- - if: '$CI_COMMIT_REF_NAME =~ /^opensbi/' # or the branch/tag starts with 'opensbi'
-   when: always
- - if: '$CI_COMMIT_MESSAGE =~ /opensbi/i' # or last commit description contains 'OpenSBI'
-   when: always
  artifacts:
    paths: # 'artifacts.zip' will contains the following files:
    - pc-bios/opensbi-riscv32-generic-fw_dynamic.bin
-- 
2.26.2



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH-for-5.2 2/2] gitlab-ci: Avoid running the EDK2 job when not necessary
  2020-11-10 12:16 [PATCH-for-5.2 0/2] gitlab-ci: Fix EDK2 & OpenSBI jobs Philippe Mathieu-Daudé
  2020-11-10 12:16 ` [PATCH-for-5.2 1/2] gitlab-ci: Avoid running the OpenSBI job when not necessary Philippe Mathieu-Daudé
@ 2020-11-10 12:16 ` Philippe Mathieu-Daudé
  2020-11-10 15:35   ` Daniel P. Berrangé
  1 sibling, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-11-10 12:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Daniel P . Berrange, Laszlo Ersek,
	Philippe Mathieu-Daudé,
	Bin Meng, Alex Bennée

The EDK2 jobs use the 'changes' keyword, which "makes it
possible to define if a job should be created based on files
modified by a Git push event." (see [1]). This keyword comes
with a warning:

  Caution:

    In pipelines with sources other than the three above
    changes can’t determine if a given file is new or old
    and always returns true."

In commit 922febe2af we moved the YAML config file from the
repository root directory to the .gitlab-ci.d/ directory.

We didn't respect the previous warning and disabled the
'changes' filter rule, as the files are now in a (directory)
three above the YAML config file.

This jobs takes ~40min, and needlessly burns the 2000 minutes
available to GitLab free users. Follow the recommendations in
[3] and disable this job by default (except if we push a tag
or the branch contains 'edk2'). Note we do not remove the job
from the pipeline, it can still be triggered manually from the
WebUI.

[1] https://docs.gitlab.com/ee/ci/yaml/#onlychangesexceptchanges
[2] https://about.gitlab.com/releases/2020/03/18/ci-minutes-for-free-users/
[3] https://about.gitlab.com/pricing/faq-consumption-cicd/

Fixes: 922febe2af ("Move edk2 and opensbi YAML files to .gitlab-ci.d folder")
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 .gitlab-ci.d/edk2.yml | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/.gitlab-ci.d/edk2.yml b/.gitlab-ci.d/edk2.yml
index e1e04524166..335c99035c9 100644
--- a/.gitlab-ci.d/edk2.yml
+++ b/.gitlab-ci.d/edk2.yml
@@ -1,10 +1,5 @@
 docker-edk2:
  stage: containers
- rules: # Only run this job when the Dockerfile is modified
- - changes:
-   - .gitlab-ci.d/edk2.yml
-   - .gitlab-ci.d/edk2/Dockerfile
-   when: always
  image: docker:19.03.1
  services:
  - docker:19.03.1-dind
@@ -26,13 +21,15 @@ docker-edk2:
 build-edk2:
  stage: build
  rules: # Only run this job when ...
- - changes: # ... roms/edk2/ is modified (submodule updated)
-   - roms/edk2/*
+ - if: $CI_COMMIT_TAG # we pushed a tag
    when: always
  - if: '$CI_COMMIT_REF_NAME =~ /^edk2/' # or the branch/tag starts with 'edk2'
    when: always
  - if: '$CI_COMMIT_MESSAGE =~ /edk2/i' # or last commit description contains 'EDK2'
    when: always
+   # ... else allow manual run on the WebUI
+ - when: manual
+   allow_failure: true
  artifacts:
    paths: # 'artifacts.zip' will contains the following files:
    - pc-bios/edk2*bz2
-- 
2.26.2



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH-for-5.2 1/2] gitlab-ci: Avoid running the OpenSBI job when not necessary
  2020-11-10 12:16 ` [PATCH-for-5.2 1/2] gitlab-ci: Avoid running the OpenSBI job when not necessary Philippe Mathieu-Daudé
@ 2020-11-10 14:33   ` Daniel P. Berrangé
  2020-11-10 15:07     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 11+ messages in thread
From: Daniel P. Berrangé @ 2020-11-10 14:33 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Alex Bennée, Thomas Huth, Bin Meng, Laszlo Ersek, qemu-devel

$SUBJECT seems to contradict the comment message and code.

Shouldn't it say  "always run the OpenSBI job"  since you're
removing all the conditional logic ?

On Tue, Nov 10, 2020 at 01:16:05PM +0100, Philippe Mathieu-Daudé wrote:
> The OpenSBI jobs use the 'changes' keyword, which "makes it
> possible to define if a job should be created based on files
> modified by a Git push event." (see [*]). This keyword comes
> with a warning:
> 
>   Caution:
> 
>     In pipelines with sources other than the three above
>     changes can’t determine if a given file is new or old
>     and always returns true."
> 
> In commit 922febe2af we moved the YAML config file from the
> repository root directory to the .gitlab-ci.d/ directory.
> 
> We didn't respect the previous warning and disabled the
> 'changes' filter rule, as the files are now in a (directory)
> three above the YAML config file.
> 
> As this job doesn't take much (less than 1min), run it by
> default.
> 
> [*] https://docs.gitlab.com/ee/ci/yaml/#onlychangesexceptchanges
> 
> Fixes: 922febe2af ("Move edk2 and opensbi YAML files to .gitlab-ci.d folder")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  .gitlab-ci.d/opensbi.yml | 13 -------------
>  1 file changed, 13 deletions(-)
> 
> diff --git a/.gitlab-ci.d/opensbi.yml b/.gitlab-ci.d/opensbi.yml
> index 5b13047e2ab..b1fa244fda4 100644
> --- a/.gitlab-ci.d/opensbi.yml
> +++ b/.gitlab-ci.d/opensbi.yml
> @@ -1,10 +1,5 @@
>  docker-opensbi:
>   stage: containers
> - rules: # Only run this job when the Dockerfile is modified
> - - changes:
> -   - .gitlab-ci.d/opensbi.yml
> -   - .gitlab-ci.d/opensbi/Dockerfile
> -   when: always
>   image: docker:19.03.1
>   services:
>   - docker:19.03.1-dind
> @@ -25,14 +20,6 @@ docker-opensbi:
>  
>  build-opensbi:
>   stage: build
> - rules: # Only run this job when ...
> - - changes: # ... roms/opensbi/ is modified (submodule updated)
> -   - roms/opensbi/*
> -   when: always
> - - if: '$CI_COMMIT_REF_NAME =~ /^opensbi/' # or the branch/tag starts with 'opensbi'
> -   when: always
> - - if: '$CI_COMMIT_MESSAGE =~ /opensbi/i' # or last commit description contains 'OpenSBI'
> -   when: always
>   artifacts:
>     paths: # 'artifacts.zip' will contains the following files:
>     - pc-bios/opensbi-riscv32-generic-fw_dynamic.bin
> -- 
> 2.26.2
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH-for-5.2 1/2] gitlab-ci: Avoid running the OpenSBI job when not necessary
  2020-11-10 14:33   ` Daniel P. Berrangé
@ 2020-11-10 15:07     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-11-10 15:07 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Alex Bennée, Thomas Huth, Bin Meng, Laszlo Ersek, qemu-devel

On 11/10/20 3:33 PM, Daniel P. Berrangé wrote:
> $SUBJECT seems to contradict the comment message and code.
> 
> Shouldn't it say  "always run the OpenSBI job"  since you're
> removing all the conditional logic ?

Sorry, bad copy/paste from previous patch.

> 
> On Tue, Nov 10, 2020 at 01:16:05PM +0100, Philippe Mathieu-Daudé wrote:
>> The OpenSBI jobs use the 'changes' keyword, which "makes it
>> possible to define if a job should be created based on files
>> modified by a Git push event." (see [*]). This keyword comes
>> with a warning:
>>
>>   Caution:
>>
>>     In pipelines with sources other than the three above
>>     changes can’t determine if a given file is new or old
>>     and always returns true."
>>
>> In commit 922febe2af we moved the YAML config file from the
>> repository root directory to the .gitlab-ci.d/ directory.
>>
>> We didn't respect the previous warning and disabled the
>> 'changes' filter rule, as the files are now in a (directory)
>> three above the YAML config file.
>>
>> As this job doesn't take much (less than 1min), run it by
>> default.
>>
>> [*] https://docs.gitlab.com/ee/ci/yaml/#onlychangesexceptchanges
>>
>> Fixes: 922febe2af ("Move edk2 and opensbi YAML files to .gitlab-ci.d folder")
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  .gitlab-ci.d/opensbi.yml | 13 -------------
>>  1 file changed, 13 deletions(-)
>>
>> diff --git a/.gitlab-ci.d/opensbi.yml b/.gitlab-ci.d/opensbi.yml
>> index 5b13047e2ab..b1fa244fda4 100644
>> --- a/.gitlab-ci.d/opensbi.yml
>> +++ b/.gitlab-ci.d/opensbi.yml
>> @@ -1,10 +1,5 @@
>>  docker-opensbi:
>>   stage: containers
>> - rules: # Only run this job when the Dockerfile is modified
>> - - changes:
>> -   - .gitlab-ci.d/opensbi.yml
>> -   - .gitlab-ci.d/opensbi/Dockerfile
>> -   when: always
>>   image: docker:19.03.1
>>   services:
>>   - docker:19.03.1-dind
>> @@ -25,14 +20,6 @@ docker-opensbi:
>>  
>>  build-opensbi:
>>   stage: build
>> - rules: # Only run this job when ...
>> - - changes: # ... roms/opensbi/ is modified (submodule updated)
>> -   - roms/opensbi/*
>> -   when: always
>> - - if: '$CI_COMMIT_REF_NAME =~ /^opensbi/' # or the branch/tag starts with 'opensbi'
>> -   when: always
>> - - if: '$CI_COMMIT_MESSAGE =~ /opensbi/i' # or last commit description contains 'OpenSBI'
>> -   when: always
>>   artifacts:
>>     paths: # 'artifacts.zip' will contains the following files:
>>     - pc-bios/opensbi-riscv32-generic-fw_dynamic.bin
>> -- 
>> 2.26.2
>>
> 
> Regards,
> Daniel
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH-for-5.2 2/2] gitlab-ci: Avoid running the EDK2 job when not necessary
  2020-11-10 12:16 ` [PATCH-for-5.2 2/2] gitlab-ci: Avoid running the EDK2 " Philippe Mathieu-Daudé
@ 2020-11-10 15:35   ` Daniel P. Berrangé
  2020-11-11  9:18     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 11+ messages in thread
From: Daniel P. Berrangé @ 2020-11-10 15:35 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Alex Bennée, Thomas Huth, Bin Meng, Laszlo Ersek, qemu-devel

On Tue, Nov 10, 2020 at 01:16:06PM +0100, Philippe Mathieu-Daudé wrote:
> The EDK2 jobs use the 'changes' keyword, which "makes it
> possible to define if a job should be created based on files
> modified by a Git push event." (see [1]). This keyword comes
> with a warning:
> 
>   Caution:
> 
>     In pipelines with sources other than the three above
>     changes can’t determine if a given file is new or old
>     and always returns true."
> 
> In commit 922febe2af we moved the YAML config file from the
> repository root directory to the .gitlab-ci.d/ directory.
> 
> We didn't respect the previous warning and disabled the
> 'changes' filter rule, as the files are now in a (directory)
> three above the YAML config file.

This description is a bit wierd. I don't see how the location
in the directory tree has any relevance here.

IIUC the caution docs quoted above are referring to what triggered 
the pipeline. They're saying that if the trigger was not a "branch", 
"merge request", or "external pull request", then the "changes" rule 
always evaluates true.

The "branch" source us a bit wierd though, as I'm not seeing
how gitlab figures out which commits are "new" to the pipeline
and thus whether the files were modified or not.

Strangely qemu-project/qemu CI for master seems to be behaving
correctly and skipping the jobs.

Something is fishy here and clearly not working, so clearly
changes are needed, but the commit message is not explaining
it for me.

> This jobs takes ~40min, and needlessly burns the 2000 minutes
> available to GitLab free users. Follow the recommendations in
> [3] and disable this job by default (except if we push a tag
> or the branch contains 'edk2'). Note we do not remove the job
> from the pipeline, it can still be triggered manually from the
> WebUI.
> 
> [1] https://docs.gitlab.com/ee/ci/yaml/#onlychangesexceptchanges
> [2] https://about.gitlab.com/releases/2020/03/18/ci-minutes-for-free-users/
> [3] https://about.gitlab.com/pricing/faq-consumption-cicd/
> 
> Fixes: 922febe2af ("Move edk2 and opensbi YAML files to .gitlab-ci.d folder")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  .gitlab-ci.d/edk2.yml | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/.gitlab-ci.d/edk2.yml b/.gitlab-ci.d/edk2.yml
> index e1e04524166..335c99035c9 100644
> --- a/.gitlab-ci.d/edk2.yml
> +++ b/.gitlab-ci.d/edk2.yml
> @@ -1,10 +1,5 @@
>  docker-edk2:
>   stage: containers
> - rules: # Only run this job when the Dockerfile is modified
> - - changes:
> -   - .gitlab-ci.d/edk2.yml
> -   - .gitlab-ci.d/edk2/Dockerfile
> -   when: always
>   image: docker:19.03.1
>   services:
>   - docker:19.03.1-dind
> @@ -26,13 +21,15 @@ docker-edk2:
>  build-edk2:
>   stage: build
>   rules: # Only run this job when ...
> - - changes: # ... roms/edk2/ is modified (submodule updated)
> -   - roms/edk2/*
> + - if: $CI_COMMIT_TAG # we pushed a tag
>     when: always
>   - if: '$CI_COMMIT_REF_NAME =~ /^edk2/' # or the branch/tag starts with 'edk2'
>     when: always
>   - if: '$CI_COMMIT_MESSAGE =~ /edk2/i' # or last commit description contains 'EDK2'
>     when: always
> +   # ... else allow manual run on the WebUI
> + - when: manual
> +   allow_failure: true
>   artifacts:
>     paths: # 'artifacts.zip' will contains the following files:
>     - pc-bios/edk2*bz2
> -- 
> 2.26.2
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH-for-5.2 2/2] gitlab-ci: Avoid running the EDK2 job when not necessary
  2020-11-10 15:35   ` Daniel P. Berrangé
@ 2020-11-11  9:18     ` Philippe Mathieu-Daudé
  2021-01-17 18:48       ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-11-11  9:18 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Alex Bennée, Thomas Huth, Bin Meng, Laszlo Ersek, qemu-devel

On 11/10/20 4:35 PM, Daniel P. Berrangé wrote:
> On Tue, Nov 10, 2020 at 01:16:06PM +0100, Philippe Mathieu-Daudé wrote:
>> The EDK2 jobs use the 'changes' keyword, which "makes it
>> possible to define if a job should be created based on files
>> modified by a Git push event." (see [1]). This keyword comes
>> with a warning:
>>
>>   Caution:
>>
>>     In pipelines with sources other than the three above
>>     changes can’t determine if a given file is new or old
>>     and always returns true."
>>
>> In commit 922febe2af we moved the YAML config file from the
>> repository root directory to the .gitlab-ci.d/ directory.
>>
>> We didn't respect the previous warning and disabled the
>> 'changes' filter rule, as the files are now in a (directory)
>> three above the YAML config file.
> 
> This description is a bit wierd. I don't see how the location
> in the directory tree has any relevance here.
> 
> IIUC the caution docs quoted above are referring to what triggered 
> the pipeline. They're saying that if the trigger was not a "branch", 
> "merge request", or "external pull request", then the "changes" rule 
> always evaluates true.
> 
> The "branch" source us a bit wierd though, as I'm not seeing
> how gitlab figures out which commits are "new" to the pipeline
> and thus whether the files were modified or not.
> 
> Strangely qemu-project/qemu CI for master seems to be behaving
> correctly and skipping the jobs.

What about this one?

https://gitlab.com/berrange/qemu/-/jobs/827459510

> 
> Something is fishy here and clearly not working, so clearly
> changes are needed, but the commit message is not explaining
> it for me.
> 
>> This jobs takes ~40min, and needlessly burns the 2000 minutes
>> available to GitLab free users. Follow the recommendations in
>> [3] and disable this job by default (except if we push a tag
>> or the branch contains 'edk2'). Note we do not remove the job
>> from the pipeline, it can still be triggered manually from the
>> WebUI.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH-for-5.2 2/2] gitlab-ci: Avoid running the EDK2 job when not necessary
  2020-11-11  9:18     ` Philippe Mathieu-Daudé
@ 2021-01-17 18:48       ` Philippe Mathieu-Daudé
  2021-01-18  8:30         ` Thomas Huth
  0 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-17 18:48 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Thomas Huth, Laszlo Ersek, qemu-devel, Willian Rampazzo,
	Bin Meng, Alex Bennée

On 11/11/20 10:18 AM, Philippe Mathieu-Daudé wrote:
> On 11/10/20 4:35 PM, Daniel P. Berrangé wrote:
>> On Tue, Nov 10, 2020 at 01:16:06PM +0100, Philippe Mathieu-Daudé wrote:
>>> The EDK2 jobs use the 'changes' keyword, which "makes it
>>> possible to define if a job should be created based on files
>>> modified by a Git push event." (see [1]). This keyword comes
>>> with a warning:
>>>
>>>   Caution:
>>>
>>>     In pipelines with sources other than the three above
>>>     changes can’t determine if a given file is new or old
>>>     and always returns true."
>>>
>>> In commit 922febe2af we moved the YAML config file from the
>>> repository root directory to the .gitlab-ci.d/ directory.
>>>
>>> We didn't respect the previous warning and disabled the
>>> 'changes' filter rule, as the files are now in a (directory)
>>> three above the YAML config file.
>>
>> This description is a bit wierd. I don't see how the location
>> in the directory tree has any relevance here.
>>
>> IIUC the caution docs quoted above are referring to what triggered 
>> the pipeline. They're saying that if the trigger was not a "branch", 
>> "merge request", or "external pull request", then the "changes" rule 
>> always evaluates true.
>>
>> The "branch" source us a bit wierd though, as I'm not seeing
>> how gitlab figures out which commits are "new" to the pipeline
>> and thus whether the files were modified or not.
>>
>> Strangely qemu-project/qemu CI for master seems to be behaving
>> correctly and skipping the jobs.
> 
> What about this one?
> 
> https://gitlab.com/berrange/qemu/-/jobs/827459510

Ping?

> 
>>
>> Something is fishy here and clearly not working, so clearly
>> changes are needed, but the commit message is not explaining
>> it for me.
>>
>>> This jobs takes ~40min, and needlessly burns the 2000 minutes
>>> available to GitLab free users. Follow the recommendations in
>>> [3] and disable this job by default (except if we push a tag
>>> or the branch contains 'edk2'). Note we do not remove the job
>>> from the pipeline, it can still be triggered manually from the
>>> WebUI.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH-for-5.2 2/2] gitlab-ci: Avoid running the EDK2 job when not necessary
  2021-01-17 18:48       ` Philippe Mathieu-Daudé
@ 2021-01-18  8:30         ` Thomas Huth
  2021-01-18  9:26           ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 11+ messages in thread
From: Thomas Huth @ 2021-01-18  8:30 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Daniel P. Berrangé
  Cc: Alex Bennée, Bin Meng, Laszlo Ersek, qemu-devel, Willian Rampazzo

On 17/01/2021 19.48, Philippe Mathieu-Daudé wrote:
> On 11/11/20 10:18 AM, Philippe Mathieu-Daudé wrote:
>> On 11/10/20 4:35 PM, Daniel P. Berrangé wrote:
>>> On Tue, Nov 10, 2020 at 01:16:06PM +0100, Philippe Mathieu-Daudé wrote:
>>>> The EDK2 jobs use the 'changes' keyword, which "makes it
>>>> possible to define if a job should be created based on files
>>>> modified by a Git push event." (see [1]). This keyword comes
>>>> with a warning:
>>>>
>>>>    Caution:
>>>>
>>>>      In pipelines with sources other than the three above
>>>>      changes can’t determine if a given file is new or old
>>>>      and always returns true."
>>>>
>>>> In commit 922febe2af we moved the YAML config file from the
>>>> repository root directory to the .gitlab-ci.d/ directory.
>>>>
>>>> We didn't respect the previous warning and disabled the
>>>> 'changes' filter rule, as the files are now in a (directory)
>>>> three above the YAML config file.
>>>
>>> This description is a bit wierd. I don't see how the location
>>> in the directory tree has any relevance here.
>>>
>>> IIUC the caution docs quoted above are referring to what triggered
>>> the pipeline. They're saying that if the trigger was not a "branch",
>>> "merge request", or "external pull request", then the "changes" rule
>>> always evaluates true.
>>>
>>> The "branch" source us a bit wierd though, as I'm not seeing
>>> how gitlab figures out which commits are "new" to the pipeline
>>> and thus whether the files were modified or not.
>>>
>>> Strangely qemu-project/qemu CI for master seems to be behaving
>>> correctly and skipping the jobs.
>>
>> What about this one?
>>
>> https://gitlab.com/berrange/qemu/-/jobs/827459510
> 
> Ping?

Daniel's comment sounded like there are some changes required here, at least 
to improve the commit message? So I was expecting a v2 here. Please clarify 
Daniel's concerns, then I can either pick up this version here or the v2 (in 
case you send one).

  Thomas



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH-for-5.2 2/2] gitlab-ci: Avoid running the EDK2 job when not necessary
  2021-01-18  8:30         ` Thomas Huth
@ 2021-01-18  9:26           ` Philippe Mathieu-Daudé
  2021-01-18 10:09             ` Daniel P. Berrangé
  0 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-18  9:26 UTC (permalink / raw)
  To: Thomas Huth, Daniel P. Berrangé
  Cc: Alex Bennée, Bin Meng, Laszlo Ersek, qemu-devel, Willian Rampazzo

On 1/18/21 9:30 AM, Thomas Huth wrote:
> On 17/01/2021 19.48, Philippe Mathieu-Daudé wrote:
>> On 11/11/20 10:18 AM, Philippe Mathieu-Daudé wrote:
>>> On 11/10/20 4:35 PM, Daniel P. Berrangé wrote:
>>>> On Tue, Nov 10, 2020 at 01:16:06PM +0100, Philippe Mathieu-Daudé wrote:
>>>>> The EDK2 jobs use the 'changes' keyword, which "makes it
>>>>> possible to define if a job should be created based on files
>>>>> modified by a Git push event." (see [1]). This keyword comes
>>>>> with a warning:
>>>>>
>>>>>    Caution:
>>>>>
>>>>>      In pipelines with sources other than the three above
>>>>>      changes can’t determine if a given file is new or old
>>>>>      and always returns true."
>>>>>
>>>>> In commit 922febe2af we moved the YAML config file from the
>>>>> repository root directory to the .gitlab-ci.d/ directory.
>>>>>
>>>>> We didn't respect the previous warning and disabled the
>>>>> 'changes' filter rule, as the files are now in a (directory)
>>>>> three above the YAML config file.
>>>>
>>>> This description is a bit wierd. I don't see how the location
>>>> in the directory tree has any relevance here.
>>>>
>>>> IIUC the caution docs quoted above are referring to what triggered
>>>> the pipeline. They're saying that if the trigger was not a "branch",
>>>> "merge request", or "external pull request", then the "changes" rule
>>>> always evaluates true.
>>>>
>>>> The "branch" source us a bit wierd though, as I'm not seeing
>>>> how gitlab figures out which commits are "new" to the pipeline
>>>> and thus whether the files were modified or not.
>>>>
>>>> Strangely qemu-project/qemu CI for master seems to be behaving
>>>> correctly and skipping the jobs.
>>>
>>> What about this one?
>>>
>>> https://gitlab.com/berrange/qemu/-/jobs/827459510
>>
>> Ping?
> 
> Daniel's comment sounded like there are some changes required here, at
> least to improve the commit message? So I was expecting a v2 here.
> Please clarify Daniel's concerns, then I can either pick up this version
> here or the v2 (in case you send one).

Sorry, this "ping" was for Daniel. I'm not sure how to continue,
so shared an example in his namespace of "incorrect behavior".

Meanwhile I'm tempted to put those jobs in Manual mode (2 months now),
until we settle this with Daniel.

Thanks,

Phil.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH-for-5.2 2/2] gitlab-ci: Avoid running the EDK2 job when not necessary
  2021-01-18  9:26           ` Philippe Mathieu-Daudé
@ 2021-01-18 10:09             ` Daniel P. Berrangé
  0 siblings, 0 replies; 11+ messages in thread
From: Daniel P. Berrangé @ 2021-01-18 10:09 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Thomas Huth, Laszlo Ersek, qemu-devel, Willian Rampazzo,
	Bin Meng, Alex Bennée

On Mon, Jan 18, 2021 at 10:26:15AM +0100, Philippe Mathieu-Daudé wrote:
> On 1/18/21 9:30 AM, Thomas Huth wrote:
> > On 17/01/2021 19.48, Philippe Mathieu-Daudé wrote:
> >> On 11/11/20 10:18 AM, Philippe Mathieu-Daudé wrote:
> >>> On 11/10/20 4:35 PM, Daniel P. Berrangé wrote:
> >>>> On Tue, Nov 10, 2020 at 01:16:06PM +0100, Philippe Mathieu-Daudé wrote:
> >>>>> The EDK2 jobs use the 'changes' keyword, which "makes it
> >>>>> possible to define if a job should be created based on files
> >>>>> modified by a Git push event." (see [1]). This keyword comes
> >>>>> with a warning:
> >>>>>
> >>>>>    Caution:
> >>>>>
> >>>>>      In pipelines with sources other than the three above
> >>>>>      changes can’t determine if a given file is new or old
> >>>>>      and always returns true."
> >>>>>
> >>>>> In commit 922febe2af we moved the YAML config file from the
> >>>>> repository root directory to the .gitlab-ci.d/ directory.
> >>>>>
> >>>>> We didn't respect the previous warning and disabled the
> >>>>> 'changes' filter rule, as the files are now in a (directory)
> >>>>> three above the YAML config file.
> >>>>
> >>>> This description is a bit wierd. I don't see how the location
> >>>> in the directory tree has any relevance here.
> >>>>
> >>>> IIUC the caution docs quoted above are referring to what triggered
> >>>> the pipeline. They're saying that if the trigger was not a "branch",
> >>>> "merge request", or "external pull request", then the "changes" rule
> >>>> always evaluates true.
> >>>>
> >>>> The "branch" source us a bit wierd though, as I'm not seeing
> >>>> how gitlab figures out which commits are "new" to the pipeline
> >>>> and thus whether the files were modified or not.
> >>>>
> >>>> Strangely qemu-project/qemu CI for master seems to be behaving
> >>>> correctly and skipping the jobs.
> >>>
> >>> What about this one?
> >>>
> >>> https://gitlab.com/berrange/qemu/-/jobs/827459510
> >>
> >> Ping?
> > 
> > Daniel's comment sounded like there are some changes required here, at
> > least to improve the commit message? So I was expecting a v2 here.
> > Please clarify Daniel's concerns, then I can either pick up this version
> > here or the v2 (in case you send one).
> 
> Sorry, this "ping" was for Daniel. I'm not sure how to continue,
> so shared an example in his namespace of "incorrect behavior".

Looking at pipelines than ran for me last week, I'm not seeing
ED2 being run, likewise for recent git master jobs on the main
repo.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-01-18 10:11 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-10 12:16 [PATCH-for-5.2 0/2] gitlab-ci: Fix EDK2 & OpenSBI jobs Philippe Mathieu-Daudé
2020-11-10 12:16 ` [PATCH-for-5.2 1/2] gitlab-ci: Avoid running the OpenSBI job when not necessary Philippe Mathieu-Daudé
2020-11-10 14:33   ` Daniel P. Berrangé
2020-11-10 15:07     ` Philippe Mathieu-Daudé
2020-11-10 12:16 ` [PATCH-for-5.2 2/2] gitlab-ci: Avoid running the EDK2 " Philippe Mathieu-Daudé
2020-11-10 15:35   ` Daniel P. Berrangé
2020-11-11  9:18     ` Philippe Mathieu-Daudé
2021-01-17 18:48       ` Philippe Mathieu-Daudé
2021-01-18  8:30         ` Thomas Huth
2021-01-18  9:26           ` Philippe Mathieu-Daudé
2021-01-18 10:09             ` Daniel P. Berrangé

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.