all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH manager] fix 4481: fetch changelogs for any Proxmox repository
@ 2023-01-18 13:54 Leo Nunner
  2023-01-27 10:41 ` Fabian Grünbichler
  0 siblings, 1 reply; 4+ messages in thread
From: Leo Nunner @ 2023-01-18 13:54 UTC (permalink / raw)
  To: pve-devel

This patch fixes the issue that when the user supplied any non-standard
repositories, the changelogs often wouldn't load. For example, providing
both pve-no-subscription and pbs-no-subscription broke the changelog
API, since the URL built for pbs-no-subscription was invalid.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
 PVE/API2/APT.pm | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
index 09c76545..921b55a1 100644
--- a/PVE/API2/APT.pm
+++ b/PVE/API2/APT.pm
@@ -101,10 +101,15 @@ my $get_changelog_url =sub {
 	    $base =~ s!pool/updates/!pool/!; # for security channel
 	    $changelog_url = "http://packages.debian.org/changelogs/$base/${srcpkg}_${pkgver}/changelog";
 	} elsif ($origin eq 'Proxmox') {
-	    if ($component eq 'pve-enterprise') {
-		$changelog_url = "https://enterprise.proxmox.com/debian/$base/${pkgname}_${pkgver}.changelog";
-	    } else {
-		$changelog_url = "http://download.proxmox.com/debian/$base/${pkgname}_${pkgver}.changelog";
+	    my $data = Proxmox::RS::APT::Repositories::repositories("pve");
+
+	    for my $file ($data->{files}->@*) {
+		for my $repo ($file->{repositories}->@*) {
+		    if (join(" ", $repo->{Components}->@*) eq $component) {
+			$changelog_url = $repo->{URIs}[0] . "/$base/${pkgname}_${pkgver}.changelog";
+			last;
+		    }
+		}
 	    }
 	}
     }
-- 
2.30.2





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

* Re: [pve-devel] [PATCH manager] fix 4481: fetch changelogs for any Proxmox repository
  2023-01-18 13:54 [pve-devel] [PATCH manager] fix 4481: fetch changelogs for any Proxmox repository Leo Nunner
@ 2023-01-27 10:41 ` Fabian Grünbichler
  2023-01-28 13:56   ` Thomas Lamprecht
  0 siblings, 1 reply; 4+ messages in thread
From: Fabian Grünbichler @ 2023-01-27 10:41 UTC (permalink / raw)
  To: Proxmox VE development discussion

On January 18, 2023 2:54 pm, Leo Nunner wrote:
> This patch fixes the issue that when the user supplied any non-standard
> repositories, the changelogs often wouldn't load. For example, providing
> both pve-no-subscription and pbs-no-subscription broke the changelog
> API, since the URL built for pbs-no-subscription was invalid.
> 
> Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
> ---
>  PVE/API2/APT.pm | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
> index 09c76545..921b55a1 100644
> --- a/PVE/API2/APT.pm
> +++ b/PVE/API2/APT.pm
> @@ -101,10 +101,15 @@ my $get_changelog_url =sub {
>  	    $base =~ s!pool/updates/!pool/!; # for security channel
>  	    $changelog_url = "http://packages.debian.org/changelogs/$base/${srcpkg}_${pkgver}/changelog";
>  	} elsif ($origin eq 'Proxmox') {
> -	    if ($component eq 'pve-enterprise') {
> -		$changelog_url = "https://enterprise.proxmox.com/debian/$base/${pkgname}_${pkgver}.changelog";
> -	    } else {
> -		$changelog_url = "http://download.proxmox.com/debian/$base/${pkgname}_${pkgver}.changelog";
> +	    my $data = Proxmox::RS::APT::Repositories::repositories("pve");
> +
> +	    for my $file ($data->{files}->@*) {
> +		for my $repo ($file->{repositories}->@*) {
> +		    if (join(" ", $repo->{Components}->@*) eq $component) {

a few improvements possible here:
- it should be enough that one of the components matches (e.g., I could have
pvetest and pve-no-subscription configured in a single entry)
- this should only take enabled repositories into account
- we should probably also compare the 'Site' member of $pkgfile toe the
repository URL

since $origin and $component also come from $pkgfile at the call sites, we could
maybe just pass in $pkgfile?

other than that, this looks okay to me since our components all contain the
product, so this allows differentiation :)

> +			$changelog_url = $repo->{URIs}[0] . "/$base/${pkgname}_${pkgver}.changelog";
> +			last;
> +		    }
> +		}
>  	    }
>  	}
>      }
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 




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

* Re: [pve-devel] [PATCH manager] fix 4481: fetch changelogs for any Proxmox repository
  2023-01-27 10:41 ` Fabian Grünbichler
@ 2023-01-28 13:56   ` Thomas Lamprecht
  2023-01-30 10:53     ` Leo Nunner
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Lamprecht @ 2023-01-28 13:56 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fabian Grünbichler

Am 27/01/2023 um 11:41 schrieb Fabian Grünbichler:
> On January 18, 2023 2:54 pm, Leo Nunner wrote:
>> This patch fixes the issue that when the user supplied any non-standard
>> repositories, the changelogs often wouldn't load. For example, providing
>> both pve-no-subscription and pbs-no-subscription broke the changelog
>> API, since the URL built for pbs-no-subscription was invalid.
>>
>> Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
>> ---
>>  PVE/API2/APT.pm | 13 +++++++++----
>>  1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
>> index 09c76545..921b55a1 100644
>> --- a/PVE/API2/APT.pm
>> +++ b/PVE/API2/APT.pm
>> @@ -101,10 +101,15 @@ my $get_changelog_url =sub {
>>  	    $base =~ s!pool/updates/!pool/!; # for security channel
>>  	    $changelog_url = "http://packages.debian.org/changelogs/$base/${srcpkg}_${pkgver}/changelog";
>>  	} elsif ($origin eq 'Proxmox') {
>> -	    if ($component eq 'pve-enterprise') {
>> -		$changelog_url = "https://enterprise.proxmox.com/debian/$base/${pkgname}_${pkgver}.changelog";
>> -	    } else {
>> -		$changelog_url = "http://download.proxmox.com/debian/$base/${pkgname}_${pkgver}.changelog";
>> +	    my $data = Proxmox::RS::APT::Repositories::repositories("pve");
>> +
>> +	    for my $file ($data->{files}->@*) {
>> +		for my $repo ($file->{repositories}->@*) {
>> +		    if (join(" ", $repo->{Components}->@*) eq $component) {
> 
> a few improvements possible here:
> - it should be enough that one of the components matches (e.g., I could have
> pvetest and pve-no-subscription configured in a single entry)
> - this should only take enabled repositories into account
> - we should probably also compare the 'Site' member of $pkgfile toe the
> repository URL
> 
> since $origin and $component also come from $pkgfile at the call sites, we could
> maybe just pass in $pkgfile?
> 
> other than that, this looks okay to me since our components all contain the
> product, so this allows differentiation :)

just to be sure: is ceph with it's `test` and `main` covered too?




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

* Re: [pve-devel] [PATCH manager] fix 4481: fetch changelogs for any Proxmox repository
  2023-01-28 13:56   ` Thomas Lamprecht
@ 2023-01-30 10:53     ` Leo Nunner
  0 siblings, 0 replies; 4+ messages in thread
From: Leo Nunner @ 2023-01-30 10:53 UTC (permalink / raw)
  To: Proxmox VE development discussion, Thomas Lamprecht,
	Fabian Grünbichler

On 2023-01-28 14:56, Thomas Lamprecht wrote:
> Am 27/01/2023 um 11:41 schrieb Fabian Grünbichler:
>> a few improvements possible here:
>> - it should be enough that one of the components matches (e.g., I could have
>> pvetest and pve-no-subscription configured in a single entry)
>> - this should only take enabled repositories into account
>> - we should probably also compare the 'Site' member of $pkgfile toe the
>> repository URL
>>
>> since $origin and $component also come from $pkgfile at the call sites, we could
>> maybe just pass in $pkgfile?
>>
>> other than that, this looks okay to me since our components all contain the
>> product, so this allows differentiation :)
> just to be sure: is ceph with it's `test` and `main` covered too?

From what I can tell: yes, it is. Testing on pacific with the 'test'
repository enabled I get

$base = "dists/bullseye/test/binary-amd64"
$repo->{URIs}[0] = "http://download.proxmox.com/debian/ceph-pacific"

which results in the valid URL

http://download.proxmox.com/debian/ceph-pacific/dists/bullseye/test/binary-amd64/





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

end of thread, other threads:[~2023-01-30 10:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-18 13:54 [pve-devel] [PATCH manager] fix 4481: fetch changelogs for any Proxmox repository Leo Nunner
2023-01-27 10:41 ` Fabian Grünbichler
2023-01-28 13:56   ` Thomas Lamprecht
2023-01-30 10:53     ` Leo Nunner

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.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal