|
Description
Petteri Räty (RETIRED)
2009-11-08 15:52:10 UTC
Created attachment 228129 [details, diff]
Patch to add check for package name and version in SRC_URI.
Created attachment 228131 [details, diff]
Patch to add check for package name and version in SRC_URI and a bit of tests.
Created attachment 228133 [details, diff]
Patch to remove tests after aplying previous patch.
There are no tests for repoman currently.
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"
...
(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). 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).
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 Joachim, very cool to have you working on this. To really get it in to repoman I would recommend meeting zmedico on Freenode IRC. (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. Created attachment 229023 [details, diff]
Checks each line if it contains SRC_URI it checks if the line contains something similar to (expanded) $P.
Created attachment 229025 [details]
And some tests to check it does, what it should.
Created attachment 229027 [details, diff]
And some tests to check it does, what it should.
Ouch, didn't stop badly typed attachment in time :/ 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. Created attachment 229205 [details, diff]
Now it uses one regexp and handles comments properly.
Created attachment 229207 [details, diff]
And tests for previuous patch.
Created attachment 229427 [details, diff]
New improved check ( case-sensitive).
Created attachment 229429 [details, diff]
Tests for check.
Created attachment 229431 [details, diff]
Right this time I hope...
Created attachment 229433 [details, diff]
;(
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) Zac, I take it this still needs to be ported to the new repoman system? (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. repoman support has been removed per bug 835013. Please file a new bug (or, I suppose, reopen this one) if you feel this check is still applicable to pkgcheck and doesn't already exist. I don't recall if pkgcheck handles this, I think no? I disagree with the premise of this bug: writing the full name of the file in SRC_URI is perfectly valid and should not be "fatal". I can see use case for this as well — say, fetching patch via SRC_URI. If the patch is unlikely to change in the next version, you don't want ${P} there.
|
