Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 292394 - [PATCH] repoman: having the value of $P in SRC_URI without using $P for it should be fatal
Summary: [PATCH] repoman: having the value of $P in SRC_URI without using $P for it sh...
Status: CONFIRMED
Alias: None
Product: Portage Development
Classification: Unclassified
Component: Repoman (show other bugs)
Hardware: All Linux
: High normal with 2 votes (vote)
Assignee: Portage team
URL:
Whiteboard:
Keywords: Inclusion, PATCH
Depends on:
Blocks:
 
Reported: 2009-11-08 15:52 UTC by Petteri Räty (RETIRED)
Modified: 2020-09-11 21:58 UTC (History)
2 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
Patch to add check for package name and version in SRC_URI. (0001-Added-check-for-P-in-SRC_URI-no-tests-for-check.patch,1.11 KB, patch)
2010-04-17 12:29 UTC, Joachim Bartosik (RETIRED)
Details | Diff
Patch to add check for package name and version in SRC_URI and a bit of tests. (0001-Added-check-for-P-in-SRC_URI-to-repoman.patch,4.28 KB, patch)
2010-04-17 12:30 UTC, Joachim Bartosik (RETIRED)
Details | Diff
Patch to remove tests after aplying previous patch. (0002-Removed-tests-since-there-are-no-tests-for-repoman-n.patch,3.01 KB, patch)
2010-04-17 12:30 UTC, Joachim Bartosik (RETIRED)
Details | Diff
Checks each line if it contains SRC_URI it checks if the line contains something similar to (expanded) $P. (0001-Check-to-repoman.patch,1.49 KB, patch)
2010-04-24 20:00 UTC, Joachim Bartosik (RETIRED)
Details | Diff
And some tests to check it does, what it should. (0002-Tests-for-added-check.patch,3.02 KB, text/plain)
2010-04-24 20:01 UTC, Joachim Bartosik (RETIRED)
Details
And some tests to check it does, what it should. (0002-Tests-for-added-check.patch,3.02 KB, patch)
2010-04-24 20:01 UTC, Joachim Bartosik (RETIRED)
Details | Diff
Now it uses one regexp and handles comments properly. (0001-Add-check-to-repoman-P-not-SRC_URI.patch,1.47 KB, patch)
2010-04-26 09:57 UTC, Joachim Bartosik (RETIRED)
Details | Diff
And tests for previuous patch. (0002-Tests.patch,2.36 KB, patch)
2010-04-26 09:59 UTC, Joachim Bartosik (RETIRED)
Details | Diff
New improved check ( case-sensitive). (0001-Add-check-to-repoman-P-not-SRC_URI.patch,1.47 KB, patch)
2010-04-27 18:21 UTC, Joachim Bartosik (RETIRED)
Details | Diff
Tests for check. (0002-Tests.patch,2.36 KB, patch)
2010-04-27 18:22 UTC, Joachim Bartosik (RETIRED)
Details | Diff
Right this time I hope... (0001-Repoman-check-not-in-SRC_URI.patch,1.45 KB, patch)
2010-04-27 18:24 UTC, Joachim Bartosik (RETIRED)
Details | Diff
;( (0002-tests.patch,2.36 KB, patch)
2010-04-27 18:25 UTC, Joachim Bartosik (RETIRED)
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Petteri Räty (RETIRED) gentoo-dev 2009-11-08 15:52:10 UTC
In ebuild foomatic-1:
SRC_URI="http://foobar/foomatic-1.tar.gz"
repoman should fail if you match the expanded value of $P
Comment 1 Joachim Bartosik (RETIRED) gentoo-dev 2010-04-17 12:29:19 UTC
Created attachment 228129 [details, diff]
Patch to add check for package name and version in SRC_URI.
Comment 2 Joachim Bartosik (RETIRED) gentoo-dev 2010-04-17 12:30:15 UTC
Created attachment 228131 [details, diff]
Patch to add check for package name and version in SRC_URI and a bit of tests.
Comment 3 Joachim Bartosik (RETIRED) gentoo-dev 2010-04-17 12:30:56 UTC
Created attachment 228133 [details, diff]
Patch to remove tests after aplying previous patch.

There are no tests for repoman currently.
Comment 4 SpanKY gentoo-dev 2010-04-19 23:30:37 UTC
umm, why exactly is $P in SRC_URI invalid ?  this summary makes no sense -- $P in SRC_URI is perfectly valid, and plenty of packages use it.

$ cd /usr/portage
$ grep -R SRC_URI.*'${P}' .
...
sys-apps/openrc/openrc-0.6.1-r1.ebuild: SRC_URI="http://roy.marples.name/downloads/${PN}/${P}.tar.bz2"
...
sys-apps/x86info/x86info-1.25.ebuild:SRC_URI="http://www.codemonkey.org.uk/projects/x86info/${P}.tgz"
...
Comment 5 Sebastian Pipping gentoo-dev 2010-04-20 00:55:32 UTC
(In reply to comment #4)
> umm, why exactly is $P in SRC_URI invalid ?  this summary makes no sense -- $P
> in SRC_URI is perfectly valid, and plenty of packages use it.
> 
> $ cd /usr/portage
> $ grep -R SRC_URI.*'${P}' .

betelgeuse is referring to the value of ${P} not the string "${P}".
latter is good (agreeing with you), latter is bad (agreeing with betelgeuse).
Comment 6 Sebastian Pipping gentoo-dev 2010-04-20 00:57:51 UTC
one more try (correcting myself):

betelgeuse is referring to the value of ${P} not the string '${P}'.
latter is good (agreeing with you), former is bad (agreeing with betelgeuse).
Comment 7 SpanKY gentoo-dev 2010-04-20 02:34:11 UTC
oh, so with his original example, an ebuild that literally has:
  SRC_URI="http://foobar/foomatic-1.tar.gz"

should get a warning to use something like:
  SRC_URI="http://foobar/${P}.tar.gz"

thanks for the clarification
Comment 8 Sebastian Pipping gentoo-dev 2010-04-20 13:26:30 UTC
Joachim, very cool to have you working on this.
To really get it in to repoman I would recommend meeting zmedico on Freenode IRC.
Comment 9 Zac Medico gentoo-dev 2010-04-24 04:50:13 UTC
(In reply to comment #2)
> Created an attachment (id=228131) [details]
> Patch to add check for package name and version in SRC_URI and a bit of tests.

This only checks pkg.metadata["SRC_URI"] which has already been expanded by bash. We need it to use the check(line) method to identify the SRC_URI definition and verify that it doesn't contain $P.
Comment 10 Joachim Bartosik (RETIRED) gentoo-dev 2010-04-24 20:00:42 UTC
Created attachment 229023 [details, diff]
Checks each line if it contains SRC_URI it checks if the line contains something similar to (expanded) $P.
Comment 11 Joachim Bartosik (RETIRED) gentoo-dev 2010-04-24 20:01:36 UTC
Created attachment 229025 [details]
And some tests to check it does, what it should.
Comment 12 Joachim Bartosik (RETIRED) gentoo-dev 2010-04-24 20:01:47 UTC
Created attachment 229027 [details, diff]
And some tests to check it does, what it should.
Comment 13 Joachim Bartosik (RETIRED) gentoo-dev 2010-04-24 20:03:17 UTC
Ouch, didn't stop badly typed attachment in time :/
Comment 14 Joachim Bartosik (RETIRED) gentoo-dev 2010-04-25 00:44:38 UTC
Two things are wrong with this patch ( it's using two regexps but it could one and it doesn't check lines with SRC_URI and comment), so don't bother reading it. I'll fix it on monday.
Comment 15 Joachim Bartosik (RETIRED) gentoo-dev 2010-04-26 09:57:15 UTC
Created attachment 229205 [details, diff]
Now it uses one regexp and handles comments properly.
Comment 16 Joachim Bartosik (RETIRED) gentoo-dev 2010-04-26 09:59:08 UTC
Created attachment 229207 [details, diff]
And tests for previuous patch.
Comment 17 Joachim Bartosik (RETIRED) gentoo-dev 2010-04-27 18:21:48 UTC
Created attachment 229427 [details, diff]
New improved check ( case-sensitive).
Comment 18 Joachim Bartosik (RETIRED) gentoo-dev 2010-04-27 18:22:15 UTC
Created attachment 229429 [details, diff]
Tests for check.
Comment 19 Joachim Bartosik (RETIRED) gentoo-dev 2010-04-27 18:24:27 UTC
Created attachment 229431 [details, diff]
Right this time I hope...
Comment 20 Joachim Bartosik (RETIRED) gentoo-dev 2010-04-27 18:25:14 UTC
Created attachment 229433 [details, diff]
;(
Comment 21 Alex Xu (Hello71) 2015-07-02 16:38:14 UTC
wouldn't it be easier/more accurate to just check for '$' character in SRC_URI and warn if it is not found? (assuming people are aware of all variables if they know one)
Comment 22 Brian Dolbec gentoo-dev 2017-03-16 00:36:36 UTC
Zac, I take it this still needs to be ported to the new repoman system?
Comment 23 Zac Medico gentoo-dev 2020-09-11 21:58:04 UTC
(In reply to Brian Dolbec from comment #22)
> Zac, I take it this still needs to be ported to the new repoman system?

Yeah, we haven't merged a patch for this yet.