diff mbox series

classes/repository: use the proper filename to find the packages under repo

Message ID 20241116172900.897898-1-srinuvasan.a@siemens.com
State Superseded, archived
Headers show
Series classes/repository: use the proper filename to find the packages under repo | expand

Commit Message

srinuvasan.a Nov. 16, 2024, 5:28 p.m. UTC 
From: srinuvasan <srinuvasan.a@siemens.com>

The very first time when we generate the base-apt the REPO_BASE_DIR
is empty, it doesn't contain any packages, hence repo_contains_package
function returns 2, based on the return value (2), we are adding all the
packages to the empty repo by calling the repo_add_packages function.

After clearing the temporary and cache files, when we retrigger the base-apt
we should skip the repo_add_packages function for all the packages (assume that
repo packages are not contaminated), but we observed repo_contains_function
says some of the packages not available in the existing repo, but really those
packages available in the repo.

Here the issue is, reprepro caching all the packages by skipping the
epoch version if any packages have, and all the packages contain only the
<upstream_version>-<debian_version>.

But in the download folder , few packages contain the epoch version,
f.e: automake, git-man, ssh, now we try to find the packages with
epoch version in APT repo, but APT repos skip the epoch version
during repo generation.

With that wrong finding again we are calling the repo_add_package
function for few packages, even those packages already available in the repo.

Skip the epoch version during finding the packages in the repo, with that we can eliminate
unnecessarily adding the packages to repo.

Signed-off-by: srinuvasan <srinuvasan.a@siemens.com>
---
 meta/classes/repository.bbclass | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

cedric.hombourger@siemens.com Nov. 16, 2024, 5:38 p.m. UTC | #1 
Hello Srinu,

I have a different implementation of this function that I was going to
submit after doing some clean-up and running a full CI build

Here's anyhow what it looks like:

# lookup ${file} in the database for the current suite
package=$(reprepro -b ${dir} --dbdir ${dbdir} \
                   --list-format 'Filename\n' \
                   listfilter ${distro} \
                   'Filename (% /'${file##*/}')' | head -n 1)
if [ -n "$package" ]; then
    # yes
    cmp --silent "$package" "$file" && return 0

    # yes but not the exact same file
    return 1
fi
# no
return 2

I would suggest you come up with a more concise description of your
change and ideally some way to reproduce and exhibit the problem as I
am not sure I follow your problem statement

On Sat, 2024-11-16 at 22:58 +0530, srinuvasan.a@siemens.com wrote:
> From: srinuvasan <srinuvasan.a@siemens.com>
> 
> The very first time when we generate the base-apt the REPO_BASE_DIR
> is empty, it doesn't contain any packages, hence
> repo_contains_package
> function returns 2, based on the return value (2), we are adding all
> the
> packages to the empty repo by calling the repo_add_packages function.
> 
> After clearing the temporary and cache files, when we retrigger the
> base-apt
> we should skip the repo_add_packages function for all the packages
> (assume that
> repo packages are not contaminated), but we observed
> repo_contains_function
> says some of the packages not available in the existing repo, but
> really those
> packages available in the repo.
> 
> Here the issue is, reprepro caching all the packages by skipping the
> epoch version if any packages have, and all the packages contain only
> the
> <upstream_version>-<debian_version>.
> 
> But in the download folder , few packages contain the epoch version,
> f.e: automake, git-man, ssh, now we try to find the packages with
> epoch version in APT repo, but APT repos skip the epoch version
> during repo generation.
> 
> With that wrong finding again we are calling the repo_add_package
> function for few packages, even those packages already available in
> the repo.
> 
> Skip the epoch version during finding the packages in the repo, with
> that we can eliminate
> unnecessarily adding the packages to repo.
> 
> Signed-off-by: srinuvasan <srinuvasan.a@siemens.com>
> ---
>  meta/classes/repository.bbclass | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/classes/repository.bbclass
> b/meta/classes/repository.bbclass
> index 28e712fd..7379658d 100644
> --- a/meta/classes/repository.bbclass
> +++ b/meta/classes/repository.bbclass
> @@ -99,7 +99,8 @@ repo_contains_package() {
>      local file="$2"
>      local package
>  
> -    package=$(find ${dir} -name ${file##*/})
> +    file_name=$(echo "${file##*/}" | sed 's/[0-9]%3a//g')
> +    package=$(find ${dir} -name ${file_name})
>      if [ -n "$package" ]; then
>          # yes
>          cmp --silent "$package" "$file" && return 0
cedric.hombourger@siemens.com Nov. 16, 2024, 11:54 p.m. UTC | #2 
On Sat, 2024-11-16 at 22:58 +0530, srinuvasan.a@siemens.com wrote:
> From: srinuvasan <srinuvasan.a@siemens.com>
> 
> The very first time when we generate the base-apt the REPO_BASE_DIR
> is empty, it doesn't contain any packages, hence
> repo_contains_package
> function returns 2, based on the return value (2), we are adding all
> the
> packages to the empty repo by calling the repo_add_packages function.
> 
> After clearing the temporary and cache files, when we retrigger the
> base-apt
> we should skip the repo_add_packages function for all the packages
> (assume that
> repo packages are not contaminated), but we observed
> repo_contains_function
> says some of the packages not available in the existing repo, but
> really those
> packages available in the repo.
> 
> Here the issue is, reprepro caching all the packages by skipping the
> epoch version if any packages have, and all the packages contain only
> the
> <upstream_version>-<debian_version>.
> 
> But in the download folder , few packages contain the epoch version,
> f.e: automake, git-man, ssh, now we try to find the packages with
> epoch version in APT repo, but APT repos skip the epoch version
> during repo generation.

As I was not fully understanding the problem, I checked the packages
you listed and now it is much clearer:

$ find build/base-apt/local/ build/base-apt/downloads/ -name git-man\*
build/base-apt/local/apt/debian/pool/main/g/git/git-man_2.39.5-
0+deb12u1_all.deb
build/base-apt/downloads/deb/debian-bookworm/git-man_1%3a2.39.5-
0+deb12u1_all.deb

Could you reword your commit message to be more concise?

> 
> With that wrong finding again we are calling the repo_add_package
> function for few packages, even those packages already available in
> the repo.
> 
> Skip the epoch version during finding the packages in the repo, with
> that we can eliminate
> unnecessarily adding the packages to repo.
> 
> Signed-off-by: srinuvasan <srinuvasan.a@siemens.com>
> ---
>  meta/classes/repository.bbclass | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/classes/repository.bbclass
> b/meta/classes/repository.bbclass
> index 28e712fd..7379658d 100644
> --- a/meta/classes/repository.bbclass
> +++ b/meta/classes/repository.bbclass
> @@ -99,7 +99,8 @@ repo_contains_package() {
>      local file="$2"
>      local package
>  
> -    package=$(find ${dir} -name ${file##*/})
> +    file_name=$(echo "${file##*/}" | sed 's/[0-9]%3a//g')
> +    package=$(find ${dir} -name ${file_name})
>      if [ -n "$package" ]; then
>          # yes
>          cmp --silent "$package" "$file" && return 0
srinuvasan.a Nov. 17, 2024, 7:10 a.m. UTC | #3 
Thanks cedric for the review, I will refactor the commit message even more concise.

Many thanks,
Srinu

-----Original Message-----
From: Hombourger, Cedric (FT FDS CES LX) <cedric.hombourger@siemens.com> 
Sent: 17 November 2024 05:24
To: isar-users@googlegroups.com; Arjunan, Srinu (FT FDS CES LX PBU 2) <srinuvasan.a@siemens.com>
Cc: Kiszka, Jan (FT RPD CED) <jan.kiszka@siemens.com>
Subject: Re: [PATCH] classes/repository: use the proper filename to find the packages under repo

On Sat, 2024-11-16 at 22:58 +0530, srinuvasan.a@siemens.com wrote:
> From: srinuvasan <srinuvasan.a@siemens.com>
> 
> The very first time when we generate the base-apt the REPO_BASE_DIR is 
> empty, it doesn't contain any packages, hence repo_contains_package 
> function returns 2, based on the return value (2), we are adding all 
> the packages to the empty repo by calling the repo_add_packages 
> function.
> 
> After clearing the temporary and cache files, when we retrigger the 
> base-apt we should skip the repo_add_packages function for all the 
> packages (assume that repo packages are not contaminated), but we 
> observed repo_contains_function says some of the packages not 
> available in the existing repo, but really those packages available in 
> the repo.
> 
> Here the issue is, reprepro caching all the packages by skipping the 
> epoch version if any packages have, and all the packages contain only 
> the <upstream_version>-<debian_version>.
> 
> But in the download folder , few packages contain the epoch version,
> f.e: automake, git-man, ssh, now we try to find the packages with 
> epoch version in APT repo, but APT repos skip the epoch version during 
> repo generation.

As I was not fully understanding the problem, I checked the packages you listed and now it is much clearer:

$ find build/base-apt/local/ build/base-apt/downloads/ -name git-man\*
build/base-apt/local/apt/debian/pool/main/g/git/git-man_2.39.5-
0+deb12u1_all.deb
build/base-apt/downloads/deb/debian-bookworm/git-man_1%3a2.39.5-
0+deb12u1_all.deb

Could you reword your commit message to be more concise?

> 
> With that wrong finding again we are calling the repo_add_package 
> function for few packages, even those packages already available in 
> the repo.
> 
> Skip the epoch version during finding the packages in the repo, with 
> that we can eliminate unnecessarily adding the packages to repo.
> 
> Signed-off-by: srinuvasan <srinuvasan.a@siemens.com>
> ---
>  meta/classes/repository.bbclass | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/classes/repository.bbclass 
> b/meta/classes/repository.bbclass index 28e712fd..7379658d 100644
> --- a/meta/classes/repository.bbclass
> +++ b/meta/classes/repository.bbclass
> @@ -99,7 +99,8 @@ repo_contains_package() {
>      local file="$2"
>      local package
>  
> -    package=$(find ${dir} -name ${file##*/})
> +    file_name=$(echo "${file##*/}" | sed 's/[0-9]%3a//g')
> +    package=$(find ${dir} -name ${file_name})
>      if [ -n "$package" ]; then
>          # yes
>          cmp --silent "$package" "$file" && return 0

--
Cedric Hombourger
Siemens AG
www.siemens.com
diff mbox series

Patch

diff --git a/meta/classes/repository.bbclass b/meta/classes/repository.bbclass
index 28e712fd..7379658d 100644
--- a/meta/classes/repository.bbclass
+++ b/meta/classes/repository.bbclass
@@ -99,7 +99,8 @@  repo_contains_package() {
     local file="$2"
     local package
 
-    package=$(find ${dir} -name ${file##*/})
+    file_name=$(echo "${file##*/}" | sed 's/[0-9]%3a//g')
+    package=$(find ${dir} -name ${file_name})
     if [ -n "$package" ]; then
         # yes
         cmp --silent "$package" "$file" && return 0