In ebuild foomatic-1: SRC_URI="http://foobar/foomatic-1.tar.gz" repoman should fail if you match the expanded value of $P
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.
