| Message ID | ZwW_aHfS5oPHRCfE@bombadil.infradead.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [RFT] linux: fix ignore hunt for config if config-kdevops is defined | expand |
On Tue, Oct 08, 2024 at 07:25:28PM -0400, Luis Chamberlain wrote: > Commit f70bf0c0ea6b411d9 ("linux: fix heuristic with missing > configuration") added support for looking for the latest linux-next > config if your ref does not have an associated kernel configuration. > This failed to capture the special-casing of config-kdevops special > file, which let's you stick to that configuration for your builds. > > Fix this. > > Fixes: f70bf0c0ea6b411d9 ("linux: fix heuristic with missing configuration") > Reported-by: Chuck Lever <chuck.lever@oracle.com> > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> > --- > > Just requires testing. I tested; my clients pick up config-kdevops once again. However, I forgot to remove config-v6.12-rc2 (which matches the kernel being built) before the test. Seems like the test should have still picked up config-v6.12-rc2 because it's newer. Although, it's a symlink to config-kdevops. cel@renoir:~/src/kdevops/cel$ ls -lt playbooks/roles/bootlinux/templates/ | head total 12848 lrwxrwxrwx 1 cel users 14 Oct 8 16:18 config-v6.12-rc2 -> config-kdevops lrwxrwxrwx 1 cel users 20 Oct 8 13:47 config-vfs.blocksize -> config-next-20240424 -rw-r--r-- 1 cel users 179024 Oct 4 15:13 config-kdevops -rw-r--r-- 1 cel users 164744 Jun 5 13:46 config-v6.9 Upshot is, I'm not sure what I tested or what behavior you expect in this case? Let me know, and I can adjust and test again. > playbooks/roles/bootlinux/tasks/main.yml | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/playbooks/roles/bootlinux/tasks/main.yml b/playbooks/roles/bootlinux/tasks/main.yml > index 9606d953a521..aab672cd9073 100644 > --- a/playbooks/roles/bootlinux/tasks/main.yml > +++ b/playbooks/roles/bootlinux/tasks/main.yml > @@ -260,26 +260,34 @@ > set_fact: > configs_with_dates: "{{ configs_with_dates | default([]) + [{'file': item.path, 'date': (item.path | regex_search('config-next-(\\d{8})')).split('-')[-1]}] }}" > loop: "{{ found_configs.files }}" > - when: item.path is search('config-next-(\\d{8})') > + when: > + - not kernel_config.stat.exists > + - item.path is search('config-next-(\\d{8})') > no_log: true > delegate_to: localhost > > - name: Sort configs based on date extracted from filename > set_fact: > sorted_configs: "{{ configs_with_dates | selectattr('date', 'defined') | sort(attribute='date', reverse=True) | map(attribute='file') | list }}" > - when: configs_with_dates | length > 0 > + when: > + - not kernel_config.stat.exists > + - configs_with_dates | length > 0 > delegate_to: localhost > > - name: Set latest linux-next config if configs are found > set_fact: > latest_linux_next_config: "{{ sorted_configs[0] }}" > - when: sorted_configs | length > 0 > + when: > + - not kernel_config.stat.exists > + - sorted_configs | length > 0 > delegate_to: localhost > > - name: Use the specific kernel config or fallback to the latest linux-next > set_fact: > linux_config: "{{ target_linux_config | default('') if kernel_config.stat.exists else (latest_linux_next_config | default('') | basename) }}" > - when: latest_linux_next_config is defined > + when: > + - not kernel_config.stat.exists > + - latest_linux_next_config is defined > delegate_to: localhost > > - name: Verify config used > -- > 2.43.0 > >
> On Oct 9, 2024, at 2:22 PM, Chuck Lever <chuck.lever@oracle.com> wrote: > > On Tue, Oct 08, 2024 at 07:25:28PM -0400, Luis Chamberlain wrote: >> Commit f70bf0c0ea6b411d9 ("linux: fix heuristic with missing >> configuration") added support for looking for the latest linux-next >> config if your ref does not have an associated kernel configuration. >> This failed to capture the special-casing of config-kdevops special >> file, which let's you stick to that configuration for your builds. >> >> Fix this. >> >> Fixes: f70bf0c0ea6b411d9 ("linux: fix heuristic with missing configuration") >> Reported-by: Chuck Lever <chuck.lever@oracle.com> >> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> >> --- >> >> Just requires testing. > > I tested; my clients pick up config-kdevops once again. > > However, I forgot to remove config-v6.12-rc2 (which matches the > kernel being built) before the test. Seems like the test should > have still picked up config-v6.12-rc2 because it's newer. > > Although, it's a symlink to config-kdevops. > > cel@renoir:~/src/kdevops/cel$ ls -lt playbooks/roles/bootlinux/templates/ | head > total 12848 > lrwxrwxrwx 1 cel users 14 Oct 8 16:18 config-v6.12-rc2 -> config-kdevops > lrwxrwxrwx 1 cel users 20 Oct 8 13:47 config-vfs.blocksize -> config-next-20240424 > -rw-r--r-- 1 cel users 179024 Oct 4 15:13 config-kdevops > -rw-r--r-- 1 cel users 164744 Jun 5 13:46 config-v6.9 > > Upshot is, I'm not sure what I tested or what behavior you expect in > this case? Let me know, and I can adjust and test again. Ah. Didn't work. I removed config-v6.12-rc2, and the build step picked up config-next-20240424 again. >> playbooks/roles/bootlinux/tasks/main.yml | 16 ++++++++++++---- >> 1 file changed, 12 insertions(+), 4 deletions(-) >> >> diff --git a/playbooks/roles/bootlinux/tasks/main.yml b/playbooks/roles/bootlinux/tasks/main.yml >> index 9606d953a521..aab672cd9073 100644 >> --- a/playbooks/roles/bootlinux/tasks/main.yml >> +++ b/playbooks/roles/bootlinux/tasks/main.yml >> @@ -260,26 +260,34 @@ >> set_fact: >> configs_with_dates: "{{ configs_with_dates | default([]) + [{'file': item.path, 'date': (item.path | regex_search('config-next-(\\d{8})')).split('-')[-1]}] }}" >> loop: "{{ found_configs.files }}" >> - when: item.path is search('config-next-(\\d{8})') >> + when: >> + - not kernel_config.stat.exists >> + - item.path is search('config-next-(\\d{8})') >> no_log: true >> delegate_to: localhost >> >> - name: Sort configs based on date extracted from filename >> set_fact: >> sorted_configs: "{{ configs_with_dates | selectattr('date', 'defined') | sort(attribute='date', reverse=True) | map(attribute='file') | list }}" >> - when: configs_with_dates | length > 0 >> + when: >> + - not kernel_config.stat.exists >> + - configs_with_dates | length > 0 >> delegate_to: localhost >> >> - name: Set latest linux-next config if configs are found >> set_fact: >> latest_linux_next_config: "{{ sorted_configs[0] }}" >> - when: sorted_configs | length > 0 >> + when: >> + - not kernel_config.stat.exists >> + - sorted_configs | length > 0 >> delegate_to: localhost >> >> - name: Use the specific kernel config or fallback to the latest linux-next >> set_fact: >> linux_config: "{{ target_linux_config | default('') if kernel_config.stat.exists else (latest_linux_next_config | default('') | basename) }}" >> - when: latest_linux_next_config is defined >> + when: >> + - not kernel_config.stat.exists >> + - latest_linux_next_config is defined >> delegate_to: localhost >> >> - name: Verify config used >> -- >> 2.43.0 >> >> > > -- > Chuck Lever -- Chuck Lever
On Wed, Oct 09, 2024 at 06:43:56PM +0000, Chuck Lever III wrote: > > I removed config-v6.12-rc2, and the build step picked up > config-next-20240424 again. Ah, silly me: how about with this on top: diff --git a/playbooks/roles/bootlinux/tasks/main.yml b/playbooks/roles/bootlinux/tasks/main.yml index aab672c..b30ae97 100644 --- a/playbooks/roles/bootlinux/tasks/main.yml +++ b/playbooks/roles/bootlinux/tasks/main.yml @@ -246,7 +246,7 @@ register: kernel_config delegate_to: localhost -- name: Find all linux-next configs +- name: Find all linux-next configs if a your custom config-kdevops and ref config was not found find: paths: "{{ role_path }}/templates" patterns: "config-next*" @@ -254,13 +254,16 @@ recurse: no register: found_configs delegate_to: localhost - when: not kernel_config.stat.exists + when: + - not config_kdevops.stat.exists + - not kernel_config.stat.exists - name: Extract the date from the filenames set_fact: configs_with_dates: "{{ configs_with_dates | default([]) + [{'file': item.path, 'date': (item.path | regex_search('config-next-(\\d{8})')).split('-')[-1]}] }}" loop: "{{ found_configs.files }}" when: + - not config_kdevops.stat.exists - not kernel_config.stat.exists - item.path is search('config-next-(\\d{8})') no_log: true @@ -270,6 +273,7 @@ set_fact: sorted_configs: "{{ configs_with_dates | selectattr('date', 'defined') | sort(attribute='date', reverse=True) | map(attribute='file') | list }}" when: + - not config_kdevops.stat.exists - not kernel_config.stat.exists - configs_with_dates | length > 0 delegate_to: localhost @@ -278,6 +282,7 @@ set_fact: latest_linux_next_config: "{{ sorted_configs[0] }}" when: + - not config_kdevops.stat.exists - not kernel_config.stat.exists - sorted_configs | length > 0 delegate_to: localhost @@ -286,6 +291,7 @@ set_fact: linux_config: "{{ target_linux_config | default('') if kernel_config.stat.exists else (latest_linux_next_config | default('') | basename) }}" when: + - not config_kdevops.stat.exists - not kernel_config.stat.exists - latest_linux_next_config is defined delegate_to: localhost
On Wed, Oct 09, 2024 at 05:38:49PM -0700, Luis Chamberlain wrote: > On Wed, Oct 09, 2024 at 06:43:56PM +0000, Chuck Lever III wrote: > > > > I removed config-v6.12-rc2, and the build step picked up > > config-next-20240424 again. > > Ah, silly me: > > how about with this on top: This looks similar to the "squash me" that I posted yesterday. There is one question I have, below: > diff --git a/playbooks/roles/bootlinux/tasks/main.yml b/playbooks/roles/bootlinux/tasks/main.yml > index aab672c..b30ae97 100644 > --- a/playbooks/roles/bootlinux/tasks/main.yml > +++ b/playbooks/roles/bootlinux/tasks/main.yml > @@ -246,7 +246,7 @@ > register: kernel_config > delegate_to: localhost > > -- name: Find all linux-next configs > +- name: Find all linux-next configs if a your custom config-kdevops and ref config was not found > find: > paths: "{{ role_path }}/templates" > patterns: "config-next*" > @@ -254,13 +254,16 @@ > recurse: no > register: found_configs > delegate_to: localhost > - when: not kernel_config.stat.exists > + when: > + - not config_kdevops.stat.exists > + - not kernel_config.stat.exists > > - name: Extract the date from the filenames > set_fact: > configs_with_dates: "{{ configs_with_dates | default([]) + [{'file': item.path, 'date': (item.path | regex_search('config-next-(\\d{8})')).split('-')[-1]}] }}" > loop: "{{ found_configs.files }}" > when: > + - not config_kdevops.stat.exists > - not kernel_config.stat.exists > - item.path is search('config-next-(\\d{8})') > no_log: true > @@ -270,6 +273,7 @@ > set_fact: > sorted_configs: "{{ configs_with_dates | selectattr('date', 'defined') | sort(attribute='date', reverse=True) | map(attribute='file') | list }}" > when: > + - not config_kdevops.stat.exists > - not kernel_config.stat.exists > - configs_with_dates | length > 0 > delegate_to: localhost > @@ -278,6 +282,7 @@ > set_fact: > latest_linux_next_config: "{{ sorted_configs[0] }}" > when: > + - not config_kdevops.stat.exists > - not kernel_config.stat.exists > - sorted_configs | length > 0 > delegate_to: localhost > @@ -286,6 +291,7 @@ > set_fact: > linux_config: "{{ target_linux_config | default('') if kernel_config.stat.exists else (latest_linux_next_config | default('') | basename) }}" > when: > + - not config_kdevops.stat.exists > - not kernel_config.stat.exists > - latest_linux_next_config is defined > delegate_to: localhost In this step, the new fact is based on "if kernel_config.stat.exists" -- so perhaps the "when:" clause here should not include " - not kernel_config.stat.exists" -- it looks like this step needs to run whether kernel_config.stat.exists is true or false.
On Thu, Oct 10, 2024 at 08:19:43AM -0400, Chuck Lever wrote: > On Wed, Oct 09, 2024 at 05:38:49PM -0700, Luis Chamberlain wrote: > > On Wed, Oct 09, 2024 at 06:43:56PM +0000, Chuck Lever III wrote: > > > > > > I removed config-v6.12-rc2, and the build step picked up > > > config-next-20240424 again. > > > > Ah, silly me: > > > > how about with this on top: > > This looks similar to the "squash me" that I posted yesterday. Ah sorry I had missed that! But glad we've came up with the same thing. > There is one question I have, below: > > > @@ -286,6 +291,7 @@ > > set_fact: > > linux_config: "{{ target_linux_config | default('') if kernel_config.stat.exists else (latest_linux_next_config | default('') | basename) }}" > > when: > > + - not config_kdevops.stat.exists > > - not kernel_config.stat.exists > > - latest_linux_next_config is defined > > delegate_to: localhost > > In this step, the new fact is based on "if > kernel_config.stat.exists" -- so perhaps the "when:" clause here > should not include " - not kernel_config.stat.exists" -- it looks > like this step needs to run whether kernel_config.stat.exists is > true or false. Ah, no, because kernel_config refers to the ref specific config, ie say you are on v6.12-rc3 but it does not exist, so we *don't* want to use this heuristic if the config for v6.12-rc3 was actually found. Agreed? Luis
> On Oct 10, 2024, at 7:08 PM, Luis Chamberlain <mcgrof@kernel.org> wrote: > > On Thu, Oct 10, 2024 at 08:19:43AM -0400, Chuck Lever wrote: >> On Wed, Oct 09, 2024 at 05:38:49PM -0700, Luis Chamberlain wrote: >>> On Wed, Oct 09, 2024 at 06:43:56PM +0000, Chuck Lever III wrote: >>>> >>>> I removed config-v6.12-rc2, and the build step picked up >>>> config-next-20240424 again. >>> >>> Ah, silly me: >>> >>> how about with this on top: >> >> This looks similar to the "squash me" that I posted yesterday. > > Ah sorry I had missed that! But glad we've came up with the same thing. > >> There is one question I have, below: >> >>> @@ -286,6 +291,7 @@ >>> set_fact: >>> linux_config: "{{ target_linux_config | default('') if kernel_config.stat.exists else (latest_linux_next_config | default('') | basename) }}" >>> when: >>> + - not config_kdevops.stat.exists >>> - not kernel_config.stat.exists >>> - latest_linux_next_config is defined >>> delegate_to: localhost >> >> In this step, the new fact is based on "if >> kernel_config.stat.exists" -- so perhaps the "when:" clause here >> should not include " - not kernel_config.stat.exists" -- it looks >> like this step needs to run whether kernel_config.stat.exists is >> true or false. > > Ah, no, because kernel_config refers to the ref specific config, ie say > you are on v6.12-rc3 but it does not exist, so we *don't* want to use > this heuristic if the config for v6.12-rc3 was actually found. > > Agreed? OK -- the when: is an AND not an OR. Can you fold these together and post a v2 and I will test that? -- Chuck Lever
On Fri, Oct 11, 2024 at 01:48:43PM +0000, Chuck Lever III wrote: > > > > On Oct 10, 2024, at 7:08 PM, Luis Chamberlain <mcgrof@kernel.org> wrote: > > > > On Thu, Oct 10, 2024 at 08:19:43AM -0400, Chuck Lever wrote: > >> On Wed, Oct 09, 2024 at 05:38:49PM -0700, Luis Chamberlain wrote: > >>> On Wed, Oct 09, 2024 at 06:43:56PM +0000, Chuck Lever III wrote: > >>>> > >>>> I removed config-v6.12-rc2, and the build step picked up > >>>> config-next-20240424 again. > >>> > >>> Ah, silly me: > >>> > >>> how about with this on top: > >> > >> This looks similar to the "squash me" that I posted yesterday. > > > > Ah sorry I had missed that! But glad we've came up with the same thing. > > > >> There is one question I have, below: > >> > >>> @@ -286,6 +291,7 @@ > >>> set_fact: > >>> linux_config: "{{ target_linux_config | default('') if kernel_config.stat.exists else (latest_linux_next_config | default('') | basename) }}" > >>> when: > >>> + - not config_kdevops.stat.exists > >>> - not kernel_config.stat.exists > >>> - latest_linux_next_config is defined > >>> delegate_to: localhost > >> > >> In this step, the new fact is based on "if > >> kernel_config.stat.exists" -- so perhaps the "when:" clause here > >> should not include " - not kernel_config.stat.exists" -- it looks > >> like this step needs to run whether kernel_config.stat.exists is > >> true or false. > > > > Ah, no, because kernel_config refers to the ref specific config, ie say > > you are on v6.12-rc3 but it does not exist, so we *don't* want to use > > this heuristic if the config for v6.12-rc3 was actually found. > > > > Agreed? > > OK -- the when: is an AND not an OR. Ah yes we need: diff --git a/playbooks/roles/bootlinux/tasks/main.yml b/playbooks/roles/bootlinux/tasks/main.yml index b30ae970ebba..76071b69a61f 100644 --- a/playbooks/roles/bootlinux/tasks/main.yml +++ b/playbooks/roles/bootlinux/tasks/main.yml @@ -282,8 +282,7 @@ set_fact: latest_linux_next_config: "{{ sorted_configs[0] }}" when: - - not config_kdevops.stat.exists - - not kernel_config.stat.exists + - not config_kdevops.stat.exists and not kernel_config.stat.exists - sorted_configs | length > 0 delegate_to: localhost > Can you fold these together and post a v2 and I will test that? Will do. Luis
diff --git a/playbooks/roles/bootlinux/tasks/main.yml b/playbooks/roles/bootlinux/tasks/main.yml index 9606d953a521..aab672cd9073 100644 --- a/playbooks/roles/bootlinux/tasks/main.yml +++ b/playbooks/roles/bootlinux/tasks/main.yml @@ -260,26 +260,34 @@ set_fact: configs_with_dates: "{{ configs_with_dates | default([]) + [{'file': item.path, 'date': (item.path | regex_search('config-next-(\\d{8})')).split('-')[-1]}] }}" loop: "{{ found_configs.files }}" - when: item.path is search('config-next-(\\d{8})') + when: + - not kernel_config.stat.exists + - item.path is search('config-next-(\\d{8})') no_log: true delegate_to: localhost - name: Sort configs based on date extracted from filename set_fact: sorted_configs: "{{ configs_with_dates | selectattr('date', 'defined') | sort(attribute='date', reverse=True) | map(attribute='file') | list }}" - when: configs_with_dates | length > 0 + when: + - not kernel_config.stat.exists + - configs_with_dates | length > 0 delegate_to: localhost - name: Set latest linux-next config if configs are found set_fact: latest_linux_next_config: "{{ sorted_configs[0] }}" - when: sorted_configs | length > 0 + when: + - not kernel_config.stat.exists + - sorted_configs | length > 0 delegate_to: localhost - name: Use the specific kernel config or fallback to the latest linux-next set_fact: linux_config: "{{ target_linux_config | default('') if kernel_config.stat.exists else (latest_linux_next_config | default('') | basename) }}" - when: latest_linux_next_config is defined + when: + - not kernel_config.stat.exists + - latest_linux_next_config is defined delegate_to: localhost - name: Verify config used
Commit f70bf0c0ea6b411d9 ("linux: fix heuristic with missing configuration") added support for looking for the latest linux-next config if your ref does not have an associated kernel configuration. This failed to capture the special-casing of config-kdevops special file, which let's you stick to that configuration for your builds. Fix this. Fixes: f70bf0c0ea6b411d9 ("linux: fix heuristic with missing configuration") Reported-by: Chuck Lever <chuck.lever@oracle.com> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> --- Just requires testing. playbooks/roles/bootlinux/tasks/main.yml | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)