From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1CC4A1C700D for ; Mon, 19 May 2025 13:11:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1747660296; cv=none; b=p7mhoqrLBlIXN4vJNGl01ZVSDbWEB5FYiU8AW3nOI64RaZUHO1XGotFkSkMbrO0HVXG+W9We8rvjHlpklCRCF6rTM0lycdbNrnSN61/e8NNBxiNEnqGTMB6CnakjlGK+PTlQiGeuKkWf8ou6D/M9BAnneFWG6AuxiR1+z+otfVI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1747660296; c=relaxed/simple; bh=/ylMqmRyrzGg5dm4q7xR6/VRLifiygQEw06oLe41lfI=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=A9sFycdQvKQTUy4oq8mq/XF7osSe8rcuiG1H9tP0Oq0Lv2UA4kuIYo1xwR/HC3p7lhxFZ+QM/j3Nr9IrEmxRGOlU7fIb0HzJMS/DDPT9gb0zFEVCna6tR7QgEKPRzVs5YB72fra7KlngrGoMcG6NfQD33Xhme7eglXsl43yu/TI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=SLQ9x6OY; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="SLQ9x6OY" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 88922C4CEE4; Mon, 19 May 2025 13:11:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1747660295; bh=/ylMqmRyrzGg5dm4q7xR6/VRLifiygQEw06oLe41lfI=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=SLQ9x6OYslznyVq5CbFj8bHF5qZh4142f+8iw4b9iaQVUgqNJUFuxTS7OndeOOdal 8IuoFuavmJDuB7cxtwLMW81x2BzXECWto4rJaLvYprW5um4AjzO51pI7UXrdVpHW3q hhJCy8Gn9JEz1X7rqq5dUU7iPk98ENYkdoZ2r95oyEUKJOHHqeUacVjHrtEXLnb60B Dg7uF+TgW0J3uDYjr+KbJCqk7wVNMDunyfs3GjNtfx/jVpqZgWOldjoG1YiOazmpht 3OJ5NyiIjrYC3dB6ZYfT2RnvwJaR+tC/RPR0kDeSZD+laraQ82RkjSnV39Ekkaum33 4O7zwMcSjqxgg== Message-ID: <8685ab2b-7169-4818-afb5-e7eb6cc11beb@kernel.org> Date: Mon, 19 May 2025 09:11:34 -0400 Precedence: bulk X-Mailing-List: kdevops@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1 1/2] bootlinux: Modernize install-deps/main.yml To: da.gomez@kernel.org, kdevops@lists.linux.dev Cc: Chuck Lever References: <20250518141838.176772-1-cel@kernel.org> <23720f8e-6f0f-4107-a6d5-c7cc2b9a1bdc@kernel.org> Content-Language: en-US From: Chuck Lever Organization: kernel.org In-Reply-To: <23720f8e-6f0f-4107-a6d5-c7cc2b9a1bdc@kernel.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 5/19/25 5:13 AM, Daniel Gomez wrote: > On 18/05/2025 16.18, cel@kernel.org wrote: >> From: Chuck Lever >> >> This subrole is run using include_tasks, but install-deps/main.yml >> then imports the distribution-specific steps. It's better to avoid >> mixing import_tasks and include_tasks. > > I agree. > > Ansible explains tag inheritance recommendation here: > https://docs.ansible.com/ansible/latest/playbook_guide/ > playbooks_tags.html#tag-inheritance-for-includes-blocks-and-the-apply- > keyword > > Can you add that to the commit message? Yes. >> Signed-off-by: Chuck Lever >> --- >>   .../bootlinux/tasks/install-deps/main.yml     | 21 ++++++++++++------- >>   1 file changed, 14 insertions(+), 7 deletions(-) >> >> diff --git a/playbooks/roles/bootlinux/tasks/install-deps/main.yml b/ >> playbooks/roles/bootlinux/tasks/install-deps/main.yml >> index c4c16d20509c..2931f9fe06e3 100644 >> --- a/playbooks/roles/bootlinux/tasks/install-deps/main.yml >> +++ b/playbooks/roles/bootlinux/tasks/install-deps/main.yml >> @@ -1,8 +1,15 @@ >>   --- >> -- name: oscheck distribution ospecific setup >> -  import_tasks: debian/main.yml >> -  when: ansible_facts['os_family']|lower == 'debian' >> -- import_tasks: suse/main.yml >> -  when: ansible_facts['os_family']|lower == 'suse' >> -- import_tasks: redhat/main.yml >> -  when: ansible_facts['os_family']|lower == 'redhat' >> +- name: Debian-specific setup >> +  ansible.builtin.include_tasks: debian/main.yml >> +  when: >> +    - ansible_os_family == "Debian" > > In general, I think Ansible import is preferred for simplicity. The > include_tasks module forces us to be explicit with the tags at > 'include_*' task and at every included task level. Here, no tags are > applied but debian/main.yml uses the linux tag. So if the playbook is > run without specifying a tag (i.e. through linux Makefile target), I > believe the file will be included but no tasks will be executed. Is that > correct? > The "linux" tag is not set by any invocation of this role. I have a patch that removes the tag completely. I can include that patch when I repost. I went back and forth about whether to stick with import_tasks or change to include_tasks, and I can't remember why I went with include_tasks. So right at the moment, I'm not stuck on either one or the other. -- Chuck Lever