Improving namcap

DaNiMoTh · 2007-07-14 13:42:13

Hi all

When I read this topic ( http://bbs.archlinux.org/viewtopic.php?id=35121 ) the first thing that I've thought was: Why we haven't a tool that check our PKGBUILD?

Of course I download the namcap source code, and I can't ready any control to the PKGBUILD ( for example, checking the presence of the field, or their order ). Two hours of work after launch, and I'm glad to post the first modifications to namcap!!!

But I need help to test and improve it, before sending final result to Jason Chu, Maintainer of namcap.

If you want helping us, you need to do some steps:

1) download the original source code from svn:

svn co http://projects.xennet.org/namcap/trunk/

2) apply this patch ( http://jjdanimoth.altervista.org/arch/i … amcap.diff ) to the source code.

If you want to test it, simply:

./namcap.py path/of/PKGBUILD

I'm a beginner with python, so aspect many bugs

Let's start the testing!! -- Patch or description of the problem are appreciated

-----------

What does this patch?

- Add a control for pkgname, pkgver, pkgrel, pkgdesc, arch, license, url, source field
- Add a control for the order of them

What need to do, but isn't coded?

- Control if some field are array or just a string ( example: license field is and array, if is a string namcap need to return an ERROR ) - I don't know how coding it in RegExp;
- Control if there are some empty field
- ...

Last edited by DaNiMoTh (2007-07-14 13:47:04)

shinlun · 2007-07-14 13:53:33

The diff download link seems to be broken.
Does anyone have the same problem ?

ps. it works now ^^ Thanks

Last edited by shinlun (2007-07-14 14:03:59)

DaNiMoTh · 2007-07-14 14:03:03

Try

wget http://jjdanimoth.altervista.org/arch/namcap/namcap.diff

Edit:

If you can't see the patch, go to http://jjdanimoth.altervista.org/arch with your web browser, enter in the 'namcap' directory and open the diff.

Excuse me but my host ( altervista ) use these methods to stop the 'hard-linking' from other webpage.

Last edited by DaNiMoTh (2007-07-14 15:33:12)

High|ander · 2007-07-14 14:28:55

The link to diff doesn't work for me either. I'm @ work now using windows so I can't use wget atm

PenguinFlavored · 2007-07-15 22:24:28

I hacked up some more code that addresses the following items from
http://bbs.archlinux.org/viewtopic.php?id=35121

...
2. The license field is NOT a string, it's an array. license=('GPL') is correct, license="GPL" is wrong. Again, a trivial mistake.
3. Use || return 1 on ALL make and patch lines. make || return 1. make DESTDIR=$startdir/pkg || return 1. patch -Np1 -i patch.file || return 1. I've seen many packages that error on make install, yet makepkg completes the pacakge.
...
8. When using a sourceforge link for source(), the only tld used should be downloads.sourceforge.net (or downloads.sf.net). Do NOT link to specific mirrors (and please do not use dl.sourceforge.net).

I also, am a python beginner, in fact I haven't touched python before this, so please verify I'm doing the right thing. Preliminary tests on my own PKGBUILDs seem to indicate that it works.

Here's the patch, It also includes DaNiMoTh's patch.
http://penguinflavored.com/namcap/namcap-1.diff

@DaNiMoTh
this addresses your todo list (except the '...' ).

So far, here's the ToDo:

...
4. If your license field has anything other than a common license, the license MUST be installed to $startdir/pkg/usr/share/licenses/$pkgname/LICENSE (or whatever the file is called, eg COPYING, Copyright.txt, etc). There should be no exceptions here. Common licenses can be found in /usr/share/licenses/common.
5. For everyone that uses makepkg -g >> PKGBUILD, and I'm pretty sure everyone does, please don't leave the md5sum on the last line, it takes a few keyboard strokes to put it right after source=().
6. Every PKGBUILD submitted to the AUR MUST have a Contributor field, where you specify your name (or nick) and a working email. Do NOT use Maintainer, that is reserved for devs and TUs only. There should be no exceptions to this rule.
...
9. Try to avoid using custom variables. Especially in the meta fields. They should only be used if and only if the PKGBUILD will not build without it (examples, kernel pkgbuilds). When custom variables are used, they MUST start with an _, eg. _realpkgname, _altsource, etc.

Please help test, and please let us know of any bugs / patches / features.

Thanks!

Last edited by PenguinFlavored (2007-07-17 01:04:02)

DaNiMoTh · 2007-07-16 13:48:28

Only one note: in some cases ( ex. the cvs, svn or git package ) has the source and md5sums field, but they are clean. In other, the depends field is clean.So, this part of the patch

 +        if pkginfo.depends == None or pkginfo.depends == "":
+                        ret[0].append('Missing depends field')
+                        var = True
+        if pkginfo.source == None or pkginfo.source == "":
+                        ret[0].append('Missing source field')
+                        var = True

Isn't correct; we need to remove the pkginfo.* == "" part. Do you agree?

For the poing 5, we just need control if PKGBUILD ends with a '}\n' or a '}\n\n' . Do you agree? ( At this moment I haven't time to code it, but I think that is simply to do).

The point 6, IMHO, need to be coded directly in AUR source.

PenguinFlavored · 2007-07-16 18:28:37

DaNiMoTh wrote:

Only one note: in some cases ( ex. the cvs, svn or git package ) has the source and md5sums field, but they are clean. In other, the depends field is clean.So, this part of the patch
 +        if pkginfo.depends == None or pkginfo.depends == "":
+                        ret[0].append('Missing depends field')
+                        var = True
+        if pkginfo.source == None or pkginfo.source == "":
+                        ret[0].append('Missing source field')
+                        var = True
Isn't correct; we need to remove the pkginfo.* == "" part. Do you agree?

Ok, sounds good, we should only check for required variables, and source and md5sums aren't necessarily required.

I'll remove the source check, but depends seems like it would be a required variable. As do pkgname, pkgversion, pkgrel, pkgdesc, arch, license, url. Is this correct? is there a document somewhere stating which ones *have* to be there? I couldn't find anything after a bit of searching.

DaNiMoTh wrote:

For the poing 5, we just need control if PKGBUILD ends with a '}\n' or a '}\n\n' . Do you agree? ( At this moment I haven't time to code it, but I think that is simply to do).

For point 5 my idea is to check for the end of the build function and make sure that no variables are declared after that.

DaNiMoTh wrote:

The point 6, IMHO, need to be coded directly in AUR source.

The AUR should do more validation on PKGBUILDs submitted, but that's a different topic. It can't hurt if namcap checks for this. Perhaps we could create different 'modes', so to speak, for different situations. Eg. a dev/TU mode, an AUR mode, and a non-AUR mode.

Once namcap reliably verifies a valid PKGBUILD the AUR and CVS could use it to validate incoming packages. It might even be a good idea to include it with makepkg to spit out warnings before a compile. This idea needs to be evaluated by the devs, before it happens, already I can see some problems with it. What if a warning or error needs to be overridden? How would it affect KISS? etc.

A new patch should arrive later tonight.
Here are some changes so far:
- Removed the check for the source variable
- copied DaNiMoTh's changes to tags.py to varorder.py and reverted tags.py
- added a check for the contributor tag to tags.py
- added a check for custom variables - makes sure they start with an underscore
So far that's 6 and 9 on the ToDo list

Last edited by PenguinFlavored (2007-07-16 23:42:36)

PenguinFlavored · 2007-07-17 01:10:36

Later tonight ...

Here's the latest patch:
http://penguinflavored.com/namcap/namcap-2.diff

Let me know how it works, and what bugs you find. Patches are welcome.

Changes:
Moved DaNiMoTh's changes to Namcap/varorder.py
reverted Namcap/tags.py to the original
Namcap/tags.py now checks for contributor tag
Namcap/arrays.py now checks for '|| return 1' after autogen and ./configure commands
Namcap/extravars.py checks for custom variables, and makes sure they start with an underscore
Added info for the new rules into the man page

tomk · 2007-07-17 13:24:50

DaNiMoTh wrote:

- Add a control for the order of them

Brief comment - there is no formally defined order for PKGBUILD fields, nor is there any need for one.

toofishes · 2007-07-17 13:44:21

tomk wrote:

DaNiMoTh wrote:
- Add a control for the order of them
Brief comment - there is no formally defined order for PKGBUILD fields, nor is there any need for one.

+1 on this, tomk. This is the stupidest thing I've heard. No doubt that there are suggestions for order, such as pkgname and pkgver coming first, etc. However, I have not seen this mystical document in devland that lays down the law on order. As was stated in the other thread where this all started, some people have preferences. If you are the maintainer, then do what you want. Telling someone to fix a PKGBUILD because the arch field is one spot to low is ludicrous- why don't you get on the people that haven't even updated their PKGBUILDs with arch fields?

I'm glad you are taking the initiative to update namcap a bit to check for the rest of the fields, but checking for order is just stupid IMO.

neotuli · 2007-07-17 13:49:20

It's a bash script for a reason. There is no predefined order for variables, so long as they're declared before they're used. And on that token, I'm going to start putting my build() functions on the first line from now on, assuming they don't use variables (which, on a similar token... why the FUCK would you use $pkgname instead of say... "oink" EVERYWHERE ... one is double the length of the other and it's not likely the pkgname is going to change anyway... why oh why)

DaNiMoTh · 2007-07-17 14:59:59

Thanks for the OT - http://bbs.archlinux.org/viewtopic.php? … 64#p266564

PenguinFlavored · 2007-07-17 16:20:51

IMHO, the ordering of the fields aren't that important. However I don't think namcap should stop checking for order. The severity of an out of order variable shouldn't discount a PKGBUILD from being tagged safe in the AUR. That's why the messages are warnings, not errors. Some of the rules will have to be decided on. I don't see the order as being an absolutely necessary requirement.

neotuli wrote:

It's a bash script for a reason. There is no predefined order for variables, so long as they're declared before they're used. And on that token, I'm going to start putting my build() functions on the first line from now on, assuming they don't use variables (which, on a similar token... why the FUCK would you use $pkgname instead of say... "oink" EVERYWHERE ... one is double the length of the other and it's not likely the pkgname is going to change anyway... why oh why)

Calm down. The reason we have variables named pkgname instead of oink is for readability. I knew a guy who didn't like his required intro to programming project, so he named all the variables after his ex-girlfriends. It was amusing, and it compiled and ran, but it took forever to understand what it was doing. Same reason people like to put the important variables at the top, the _head_ers should be at the _head_ of the document.

toofishes wrote:

However, I have not seen this mystical document in devland that lays down the law on order.

PKGBUILD.proto has the order of the variables, but it's not law.

tomk · 2007-07-18 11:38:13

Another brief comment (hopefully OT this time): it seems to me that what you want is a tool for TUs to assist with AUR package flagging. I can see how this would be very useful, but I would also see it as something completely separate from namcap. So I'd suggest you take what you've done so far and use it as the basis for a new project, to provide the functions you need.

iphitus · 2007-07-18 11:43:47

neotuli wrote:

It's a bash script for a reason. There is no predefined order for variables, so long as they're declared before they're used. And on that token, I'm going to start putting my build() functions on the first line from now on, assuming they don't use variables (which, on a similar token... why the FUCK would you use $pkgname instead of say... "oink" EVERYWHERE ... one is double the length of the other and it's not likely the pkgname is going to change anyway... why oh why)

amen to that. finally good to see someone else realise how pointless $pkgname in build {} is.

DaNiMoTh · 2007-07-18 19:41:30

tomk wrote:

Another brief comment (hopefully OT this time): it seems to me that what you want is a tool for TUs to assist with AUR package flagging. I can see how this would be very useful, but I would also see it as something completely separate from namcap. So I'd suggest you take what you've done so far and use it as the basis for a new project, to provide the functions you need.

Not at all.. because the namcap that I've in mind does help, before all, the final users that make their pkgbuild. It remind some situations ( like wrong md5sums, etc ) which, in some cases, can escape. The form of PKGBUILD is secondary ( and I remind that all case of PKGBUILD wrong form, are segnalated by a warning ).

Dusty · 2007-07-18 20:00:03

neotuli wrote:

it's not likely the pkgname is going to change anyway...

GAIM --> Pidgen!!!!!!!!!!

I know I know, I'm a wiseass.

Dusty

PenguinFlavored · 2007-07-18 20:02:25

tomk wrote:

Another brief comment (hopefully OT this time): it seems to me that what you want is a tool for TUs to assist with AUR package flagging. I can see how this would be very useful, but I would also see it as something completely separate from namcap. So I'd suggest you take what you've done so far and use it as the basis for a new project, to provide the functions you need.

Correct me if I'm wrong, but doesn't namcap verify the correctness of the package

$man namcap
namcap is a package analysis utility that looks for problems with arch-
linux packages or their PKGBUILD files.

Yes, this is a useful tool for TUs to flag packages. As it would be useful for people who want to submit to the AUR. Why make the TU's jobs harder by having to check packages that the contributors haven't checked themselves? I can see namcap being useful for all kinds of package verification, even including it in the AUR source, and doing audits on the official repos.

STiAT · 2007-07-19 09:25:13

What would be really helpful:

- Checking for Perl files
- Checking for Python files
- Checking for Ruby files
- Checking for TCL/TK
- Accepting .CHANGELOG

I use namcap usually to verify packages. It's a nice tool, i like it a lot. I don't see the reason for any order though, or for empty variables, i use comment that and flag them safe anyway ... since "safe" doesn't mean "perfect" in the way the pkgbuild is written to me, just that it won't harm the system, includes all necessary build information (in example deps) and works properly.

I wouldn't even say mine are written as suggested. I've a pkgbuild template i like and use, most likely structured by "information", package information (name, num, rel, desc, url), build information (arch, deps, makedeps), file information (install, source, md5sum etc), build script.

I havn't seen anyone complaining about this right now, and i don't see the point on changing it. If i give up maintaining on packages, and someone else wants to adobt them, i think it's acceptable for the one to change the things in the order he/she likes it.

Yours,
Georg

DaNiMoTh · 2007-07-19 11:37:05

What do you mean with 'Checking' ?

----

Penguin: I think that we need to remove the Namcap/varorder.py, then all is ready for mailing Jason. Do you agree?

STiAT · 2007-07-20 09:07:47

The case is that namcap always complains about tcl / perl / python etc are not a dependency, even if it are perl / python / tcl applications.

I often need to look therefore if that's correct, what costs some more time, what could be done by the script as well (of my point of view).

PenguinFlavored · 2007-07-21 20:39:19

Hi all,

I've removed varorder.py, it seems this is the consensus. I also fixed a bug I found. I wrap multiple lined arrays into a single line. This fixes the md5sums rules as well as array parsing and checking. So far it looks good. Test, it out and after a few days of reliability we can send it to Jason.

Hopefully people can agree on these rules. I made non-arrays that should be arrays into an error instead of a warning.

Here's the patch:
http://www.penguinflavored.com/namcap/namcap-3.diff
again, apply it to the namcap v1.6
svn co http://projects.xennet.org/namcap/tags/1.6

@STiAT
So you're saying scripting languages don't get detected as a dependency? After briefly looking at Namcap/depends.py it looks like it looks at the hashbang line and ruby/perl/bash/etc. Is this functionality not working? Can you provide an example PKGBUILD?

Last edited by PenguinFlavored (2007-07-21 20:42:24)

PenguinFlavored · 2007-07-24 04:59:09

The namcap repository has moved to git at http://projects.archlinux.org/git/namcap.git

DaNiMoTh · 2007-07-24 10:03:08

Now use the command

 git clone http://projects.archlinux.org/git/namcap.git

to get the source.

I tested the patch; I think that we would remove the parts for the Maintainer and Contributor tag ( If I'm a simply AUR user, why I should insert a Maintainer tag?) and the || return 1 part, waiting an official response, on ml ( there is a discussion now under tu-ml ). Your opinion?

The function that control if extra vars have an underscore don't work. Do you confirm this?

Do it, and we can post on archlinux mailing list or directly to Jason. Do you agree?

Last edited by DaNiMoTh (2007-07-24 10:04:47)

Lone_Wolf · 2007-07-24 12:10:22

PenguinFlavored wrote:

@STiAT
So you're saying scripting languages don't get detected as a dependency? After briefly looking at Namcap/depends.py it looks like it looks at the hashbang line and ruby/perl/bash/etc. Is this functionality not working? Can you provide an example PKGBUILD?

I recently encountered the same problem with the python package singularity : http://aur.archlinux.org/packages.php?d … s=0&SeB=nd

It depends on python and python-pygame , but namcap claims those are not needed.

i can understand that python-pygame may be hard to test for, but python should be detected as a necessary dep.

Last edited by Lone_Wolf (2007-07-24 12:10:43)

Arch Linux

#1 2007-07-14 13:42:13

Improving namcap

#2 2007-07-14 13:53:33

Re: Improving namcap

#3 2007-07-14 14:03:03

Re: Improving namcap

#4 2007-07-14 14:28:55

Re: Improving namcap

#5 2007-07-15 22:24:28

Re: Improving namcap

#6 2007-07-16 13:48:28

Re: Improving namcap

#7 2007-07-16 18:28:37

Re: Improving namcap

#8 2007-07-17 01:10:36

Re: Improving namcap

#9 2007-07-17 13:24:50

Re: Improving namcap

#10 2007-07-17 13:44:21

Re: Improving namcap

#11 2007-07-17 13:49:20

Re: Improving namcap

#12 2007-07-17 14:59:59

Re: Improving namcap

#13 2007-07-17 16:20:51

Re: Improving namcap

#14 2007-07-18 11:38:13

Re: Improving namcap

#15 2007-07-18 11:43:47

Re: Improving namcap

#16 2007-07-18 19:41:30

Re: Improving namcap

#17 2007-07-18 20:00:03

Re: Improving namcap

#18 2007-07-18 20:02:25

Re: Improving namcap

#19 2007-07-19 09:25:13

Re: Improving namcap

#20 2007-07-19 11:37:05

Re: Improving namcap

#21 2007-07-20 09:07:47

Re: Improving namcap

#22 2007-07-21 20:39:19

Re: Improving namcap

#23 2007-07-24 04:59:09

Re: Improving namcap

#24 2007-07-24 10:03:08

Re: Improving namcap

#25 2007-07-24 12:10:22

Re: Improving namcap

Board footer