[aur-general] Proofreading request
Hello, Could someone have a look at the attached package, please ? It's my first AUR package written from scratch. I've packaged the script Feather written in Python2 by Dan Rue to handle backups with tarsnap (https://github.com/danrue/feather). It contains some difficulties for a beginner: sources from github, a patch to correct Python executable name and a custom licence. I would be glad to have a seasoned packager looking at it. Running namcap gives two warnings: feather-git W: Dependency included and not needed ('tarsnap') feather-git W: Dependency included and not needed ('python2-yaml') since namcap probably can't notice their usage as a system call and as a Python library. Best regards, -- Clément
On Mon, Aug 26, 2013 at 11:17:16PM +0200, Clément Junca wrote:
Could someone have a look at the attached package, please ? It's my first AUR package written from scratch.
I think you attached the wrong file - this is the built package. The PKGBUILD or the src.tar.gz is what should be checked. -Jesse AKA Trilby
Yes, you're right. Sorry. Here is the good one. -- Clément 2013/8/26 Jesse McClure <jmcclure@cns.umass.edu>
On Mon, Aug 26, 2013 at 11:17:16PM +0200, Clément Junca wrote:
Could someone have a look at the attached package, please ? It's my first AUR package written from scratch.
I think you attached the wrong file - this is the built package. The PKGBUILD or the src.tar.gz is what should be checked.
-Jesse AKA Trilby
On 08/27/13 00:35, Clément Junca wrote:
Yes, you're right. Sorry. Here is the good one. You haven't attached anything to this mail.
PS: make sure you reply below the quotes in a mailing list, easier to read top to bottom. -- Taylor Lookabaugh "I consider my ability to arouse enthusiasm among my people, the greatest asset I possess, and the way to develop the best that is in a person is by appreciation and encouragement. There is nothing else that so kills the ambitions of a person as criticisms from superiors. I never criticize any-one. I believe in giving a person incentive to work. So I am anxious to praise but loath to find fault. If I like anything, I am hearty in my approbation and lavish in my praise." --Charles Schwab
2013/8/27 Taylor Lookabaugh <jesus.christ.i.love@gmail.com>
On 08/27/13 00:35, Clément Junca wrote:
Yes, you're right. Sorry. Here is the good one. You haven't attached anything to this mail.
PS: make sure you reply below the quotes in a mailing list, easier to read top to bottom.
That's strange, I see the tar.gz file in my sent mail. Here are the files from the archive.
----------------------------------------
Date: Tue, 27 Aug 2013 10:04:37 +0200 From: cju.arch@gmail.com To: aur-general@archlinux.org Subject: Re: [aur-general] Proofreading request
2013/8/27 Taylor Lookabaugh <jesus.christ.i.love@gmail.com>
On 08/27/13 00:35, Clément Junca wrote:
Yes, you're right. Sorry. Here is the good one. You haven't attached anything to this mail.
PS: make sure you reply below the quotes in a mailing list, easier to read top to bottom.
That's strange, I see the tar.gz file in my sent mail. Here are the files from the archive.
My notes: 1. Get rid of all of the empty variables (groups, provides, etc) 2. Definitely add the license file to the source array. 3. The cd .. at the end of the pkgver function is useless. 4. Applying the patch should be done in a prepare() function, you don't need a build() function at all in this case. 5. You don't need || exit 1. The functions are called in a way so it will already exit if there are errors. 6. install -D will make the dirs it needs, you don't need to make them yourself with mkdir -p. 7. The comment in the pkgver function doesn't match what it's doing, it's not using a tag. 8. If you do install the default config file, you should add it to the backup array so that pacman doesn't overwrite it every time you upgrade. I will disagree with the previous posters on a couple of things. 1. There's nothing wrong with using ../../LICENSE as long as you know what dir you're in. 2. There is nothing wrong with cd-ing directly to $_gitname, although I prefer $srcdir/$_gitname myself
On Tue, Aug 27, 2013 at 10:34:56AM -0500, Doug Newgard wrote:
----------------------------------------
Date: Tue, 27 Aug 2013 10:04:37 +0200 From: cju.arch@gmail.com To: aur-general@archlinux.org Subject: Re: [aur-general] Proofreading request
2013/8/27 Taylor Lookabaugh <jesus.christ.i.love@gmail.com>
On 08/27/13 00:35, Clément Junca wrote:
Yes, you're right. Sorry. Here is the good one. You haven't attached anything to this mail.
PS: make sure you reply below the quotes in a mailing list, easier to read top to bottom.
That's strange, I see the tar.gz file in my sent mail. Here are the files from the archive.
My notes:
1. Get rid of all of the empty variables (groups, provides, etc) 2. Definitely add the license file to the source array. 3. The cd .. at the end of the pkgver function is useless. 4. Applying the patch should be done in a prepare() function, you don't need a build() function at all in this case. 5. You don't need || exit 1. The functions are called in a way so it will already exit if there are errors. 6. install -D will make the dirs it needs, you don't need to make them yourself with mkdir -p. 7. The comment in the pkgver function doesn't match what it's doing, it's not using a tag. 8. If you do install the default config file, you should add it to the backup array so that pacman doesn't overwrite it every time you upgrade.
I will disagree with the previous posters on a couple of things.
1. There's nothing wrong with using ../../LICENSE as long as you know what dir you're in.
Yes. There is. You don't, and you can't know what directories are above you in this case. This PKGBUILD will fail when [[ -n $BUILDDIR ]] is true. More pertinent, not adding the LICENSE file to the source array means that 'makepkg -S' doesn't include this file. That the file is still included is a sign of a manually crafted source tarball and a huge red flag.
2. There is nothing wrong with cd-ing directly to $_gitname, although I prefer $srcdir/$_gitname myself
2013/8/27 Dave Reisner <d@falconindy.com>
On Tue, Aug 27, 2013 at 10:34:56AM -0500, Doug Newgard wrote:
----------------------------------------
Date: Tue, 27 Aug 2013 10:04:37 +0200 From: cju.arch@gmail.com To: aur-general@archlinux.org Subject: Re: [aur-general] Proofreading request
2013/8/27 Taylor Lookabaugh <jesus.christ.i.love@gmail.com>
On 08/27/13 00:35, Clément Junca wrote:
Yes, you're right. Sorry. Here is the good one. You haven't attached anything to this mail.
PS: make sure you reply below the quotes in a mailing list, easier to read top to bottom.
That's strange, I see the tar.gz file in my sent mail. Here are the files from the archive.
My notes:
1. Get rid of all of the empty variables (groups, provides, etc) 2. Definitely add the license file to the source array. 3. The cd .. at the end of the pkgver function is useless. 4. Applying the patch should be done in a prepare() function, you don't need a build() function at all in this case. 5. You don't need || exit 1. The functions are called in a way so it will already exit if there are errors. 6. install -D will make the dirs it needs, you don't need to make them yourself with mkdir -p. 7. The comment in the pkgver function doesn't match what it's doing, it's not using a tag. 8. If you do install the default config file, you should add it to the backup array so that pacman doesn't overwrite it every time you upgrade.
I will disagree with the previous posters on a couple of things.
1. There's nothing wrong with using ../../LICENSE as long as you know what dir you're in.
Yes. There is. You don't, and you can't know what directories are above you in this case. This PKGBUILD will fail when [[ -n $BUILDDIR ]] is true.
More pertinent, not adding the LICENSE file to the source array means that 'makepkg -S' doesn't include this file. That the file is still included is a sign of a manually crafted source tarball and a huge red flag.
2. There is nothing wrong with cd-ing directly to $_gitname, although I prefer $srcdir/$_gitname myself
Thanks a lot to all of you for theses corrections. I learned a lot. If I understand correctly putting the configuration file directly to /etc/ is fine as long as I declare it in the backup array. A few notes: * The 'backup' field is clearly explained in the wiki (or I haven't found it). * The "|| exit 1" comes from the PKGBUID of eject-unlock (not saying that to point a responsible be to allow this improvement to other packages). * /usr/share/pacman/PKGBUILD-git.proto use an explicit "git clone" and seems older than the example in the wiki.
On Tue, Aug 27, 2013 at 8:09 PM, Clément Junca <cju.arch@gmail.com> wrote:
* The 'backup' field is clearly explained in the wiki (or I haven't found it).
https://wiki.archlinux.org/index.php/PKGBUILD#backup
* The "|| exit 1" comes from the PKGBUID of eject-unlock (not saying that to point a responsible be to allow this improvement to other packages).
Better look for proto files or official package for reference. There is a lot of PKGBUILD not following latest guidelines in AUR.
* /usr/share/pacman/PKGBUILD-git.proto use an explicit "git clone" and seems older than the example in the wiki.
There is already a ticket open for this: FS#34485 Regards, -- Cédric Girard
----------------------------------------
Date: Tue, 27 Aug 2013 13:50:30 -0400 From: d@falconindy.com To: aur-general@archlinux.org Subject: Re: [aur-general] Proofreading request
On Tue, Aug 27, 2013 at 10:34:56AM -0500, Doug Newgard wrote:
----------------------------------------
Date: Tue, 27 Aug 2013 10:04:37 +0200 From: cju.arch@gmail.com To: aur-general@archlinux.org Subject: Re: [aur-general] Proofreading request
2013/8/27 Taylor Lookabaugh <jesus.christ.i.love@gmail.com>
On 08/27/13 00:35, Clément Junca wrote:
Yes, you're right. Sorry. Here is the good one. You haven't attached anything to this mail.
PS: make sure you reply below the quotes in a mailing list, easier to read top to bottom.
That's strange, I see the tar.gz file in my sent mail. Here are the files from the archive.
My notes:
1. Get rid of all of the empty variables (groups, provides, etc) 2. Definitely add the license file to the source array. 3. The cd .. at the end of the pkgver function is useless. 4. Applying the patch should be done in a prepare() function, you don't need a build() function at all in this case. 5. You don't need || exit 1. The functions are called in a way so it will already exit if there are errors. 6. install -D will make the dirs it needs, you don't need to make them yourself with mkdir -p. 7. The comment in the pkgver function doesn't match what it's doing, it's not using a tag. 8. If you do install the default config file, you should add it to the backup array so that pacman doesn't overwrite it every time you upgrade.
I will disagree with the previous posters on a couple of things.
1. There's nothing wrong with using ../../LICENSE as long as you know what dir you're in.
Yes. There is. You don't, and you can't know what directories are above you in this case. This PKGBUILD will fail when [[ -n $BUILDDIR ]] is true.
More pertinent, not adding the LICENSE file to the source array means that 'makepkg -S' doesn't include this file. That the file is still included is a sign of a manually crafted source tarball and a huge red flag.
I think you missed the part where I said "as long as you know what dir you're in". You're right that using ../../ in this case, you have no idea where you are, but that doesn't mean that using relative paths is bad, which is how I took the post I was referring to. If I misunderstood, I apologize.
2. There is nothing wrong with cd-ing directly to $_gitname, although I prefer $srcdir/$_gitname myself
On 08/26/2013 11:17 PM, Clément Junca wrote:
Hello,
Could someone have a look at the attached package, please ? It's my first AUR package written from scratch.
I've packaged the script Feather written in Python2 by Dan Rue to handle backups with tarsnap (https://github.com/danrue/feather).
It contains some difficulties for a beginner: sources from github, a patch to correct Python executable name and a custom licence. I would be glad to have a seasoned packager looking at it.
Running namcap gives two warnings: feather-git W: Dependency included and not needed ('tarsnap') feather-git W: Dependency included and not needed ('python2-yaml') since namcap probably can't notice their usage as a system call and as a Python library.
Best regards,
I would also add LICENSE to the source array, that way makepkg will symlink it into $srcdir for you (and you don't have to do fragile stuff like ../../LICENSE). Generally I'd also cd into $srcdir/$_gitname in pgkver() and build(). I'm feeling a bit uneasy about putting the config file straight to /etc, since it's clearly meant as an example. I'd maybe put it into /usr/share/$pkgname and state in the .install that the user may copy and modify it from there to where ever s/he likes it. Or as an alternative, one could not ship a config file at all and just put the README (which contains an example config) to /usr/share/doc/$pkgname and point the user to it during .install. I'll attach .diffs containing my suggestions. Then again, I'm a packaging newb myself, so I'd also like to hear from someone with experience.
On 08/27/2013 01:28 PM, ponder@creshal.de wrote:
On 08/26/2013 11:17 PM, Clément Junca wrote:
Hello,
Could someone have a look at the attached package, please ? It's my first AUR package written from scratch.
I've packaged the script Feather written in Python2 by Dan Rue to handle backups with tarsnap (https://github.com/danrue/feather).
It contains some difficulties for a beginner: sources from github, a patch to correct Python executable name and a custom licence. I would be glad to have a seasoned packager looking at it.
Running namcap gives two warnings: feather-git W: Dependency included and not needed ('tarsnap') feather-git W: Dependency included and not needed ('python2-yaml') since namcap probably can't notice their usage as a system call and as a Python library.
Best regards,
I would also add LICENSE to the source array, that way makepkg will symlink it into $srcdir for you (and you don't have to do fragile stuff like ../../LICENSE). Generally I'd also cd into $srcdir/$_gitname in pgkver() and build().
I'm feeling a bit uneasy about putting the config file straight to /etc, since it's clearly meant as an example. I'd maybe put it into /usr/share/$pkgname and state in the .install that the user may copy and modify it from there to where ever s/he likes it. Or as an alternative, one could not ship a config file at all and just put the README (which contains an example config) to /usr/share/doc/$pkgname and point the user to it during .install.
I'll attach .diffs containing my suggestions. Then again, I'm a packaging newb myself, so I'd also like to hear from someone with experience.
... I'm pretty sure I really attached them. Here they go again.
On 08/27/2013 01:43 PM, ponder@creshal.de wrote:
On 08/27/2013 01:28 PM, ponder@creshal.de wrote:
On 08/26/2013 11:17 PM, Clément Junca wrote:
Hello,
Could someone have a look at the attached package, please ? It's my first AUR package written from scratch.
I've packaged the script Feather written in Python2 by Dan Rue to handle backups with tarsnap (https://github.com/danrue/feather).
It contains some difficulties for a beginner: sources from github, a patch to correct Python executable name and a custom licence. I would be glad to have a seasoned packager looking at it.
Running namcap gives two warnings: feather-git W: Dependency included and not needed ('tarsnap') feather-git W: Dependency included and not needed ('python2-yaml') since namcap probably can't notice their usage as a system call and as a Python library.
Best regards,
I would also add LICENSE to the source array, that way makepkg will symlink it into $srcdir for you (and you don't have to do fragile stuff like ../../LICENSE). Generally I'd also cd into $srcdir/$_gitname in pgkver() and build().
I'm feeling a bit uneasy about putting the config file straight to /etc, since it's clearly meant as an example. I'd maybe put it into /usr/share/$pkgname and state in the .install that the user may copy and modify it from there to where ever s/he likes it. Or as an alternative, one could not ship a config file at all and just put the README (which contains an example config) to /usr/share/doc/$pkgname and point the user to it during .install.
I'll attach .diffs containing my suggestions. Then again, I'm a packaging newb myself, so I'd also like to hear from someone with experience.
... I'm pretty sure I really attached them. Here they go again.
Sorry for the spamming, this is really embarrassing. The attachments show up in my sent mail, but not on the list. I'll put them up at http://ponder.shurelia.de/feather-git/PKGBUILD.diff http://ponder.shurelia.de/feather-git/config.install.diff until I figure this out.
2013/8/27 <ponder@creshal.de>
On 08/27/2013 01:28 PM, ponder@creshal.de wrote:
On 08/26/2013 11:17 PM, Clément Junca wrote:
Hello,
Could someone have a look at the attached package, please ? It's my first AUR package written from scratch.
I've packaged the script Feather written in Python2 by Dan Rue to handle backups with tarsnap (https://github.com/danrue/feather).
It contains some difficulties for a beginner: sources from github, a
On 08/27/2013 01:43 PM, ponder@creshal.de wrote: patch
to correct Python executable name and a custom licence. I would be glad to have a seasoned packager looking at it.
Running namcap gives two warnings: feather-git W: Dependency included and not needed ('tarsnap') feather-git W: Dependency included and not needed ('python2-yaml') since namcap probably can't notice their usage as a system call and as a Python library.
Best regards,
I would also add LICENSE to the source array, that way makepkg will symlink it into $srcdir for you (and you don't have to do fragile stuff like ../../LICENSE). Generally I'd also cd into $srcdir/$_gitname in pgkver() and build().
I'm feeling a bit uneasy about putting the config file straight to /etc, since it's clearly meant as an example. I'd maybe put it into /usr/share/$pkgname and state in the .install that the user may copy and modify it from there to where ever s/he likes it. Or as an alternative, one could not ship a config file at all and just put the README (which contains an example config) to /usr/share/doc/$pkgname and point the user to it during .install.
I'll attach .diffs containing my suggestions. Then again, I'm a packaging newb myself, so I'd also like to hear from someone with experience.
... I'm pretty sure I really attached them. Here they go again.
Sorry for the spamming, this is really embarrassing. The attachments show up in my sent mail, but not on the list. I'll put them up at http://ponder.shurelia.de/feather-git/PKGBUILD.diff http://ponder.shurelia.de/feather-git/config.install.diff until I figure this out.
It's the same problem I had with my .tar.gz file. Can it come from the list manager software ? I will add the LICENSE to the source array. I also agree regarding the configuration file. I prefer the /usr/share solution but I can use the /u/s/doc solution if someone thinks it's a clearer solution.
On 2013-08-27 14:44, Clément Junca wrote:
2013/8/27 <ponder@creshal.de>
On 08/27/2013 01:28 PM, ponder@creshal.de wrote:
On 08/26/2013 11:17 PM, Clément Junca wrote:
Hello,
Could someone have a look at the attached package, please ? It's my first AUR package written from scratch.
I've packaged the script Feather written in Python2 by Dan Rue to handle backups with tarsnap (https://github.com/danrue/feather).
It contains some difficulties for a beginner: sources from github, a
On 08/27/2013 01:43 PM, ponder@creshal.de wrote: patch
to correct Python executable name and a custom licence. I would be glad to have a seasoned packager looking at it.
Running namcap gives two warnings: feather-git W: Dependency included and not needed ('tarsnap') feather-git W: Dependency included and not needed ('python2-yaml') since namcap probably can't notice their usage as a system call and as a Python library.
Best regards,
I would also add LICENSE to the source array, that way makepkg will symlink it into $srcdir for you (and you don't have to do fragile stuff like ../../LICENSE). Generally I'd also cd into $srcdir/$_gitname in pgkver() and build().
I'm feeling a bit uneasy about putting the config file straight to /etc, since it's clearly meant as an example. I'd maybe put it into /usr/share/$pkgname and state in the .install that the user may copy and modify it from there to where ever s/he likes it. Or as an alternative, one could not ship a config file at all and just put the README (which contains an example config) to /usr/share/doc/$pkgname and point the user to it during .install.
I'll attach .diffs containing my suggestions. Then again, I'm a packaging newb myself, so I'd also like to hear from someone with experience.
... I'm pretty sure I really attached them. Here they go again.
Sorry for the spamming, this is really embarrassing. The attachments show up in my sent mail, but not on the list. I'll put them up at http://ponder.shurelia.de/feather-git/PKGBUILD.diff http://ponder.shurelia.de/feather-git/config.install.diff until I figure this out.
It's the same problem I had with my .tar.gz file. Can it come from the list manager software ?
I will add the LICENSE to the source array.
I also agree regarding the configuration file. I prefer the /usr/share solution but I can use the /u/s/doc solution if someone thinks it's a clearer solution.
Hey there, I included ponders changes, cleaned up quite a lot in the PKGBUILD and added some "". You are not supposed to have empty variables specified in it. Also, I renamed build() to prepare() and changed the diff format to git (better visual diff). There is no need to change the directory e.g. in pkgver() when you don't need to work in that function anymore. Diffs for your originally uploaded files: https://w.tf-w.tf/aur/feather-git/PKGBUILD.patch https://w.tf-w.tf/aur/feather-git/config.install.patch New files: https://w.tf-w.tf/aur/feather-git/PKGBUILD https://w.tf-w.tf/aur/feather-git/config.install -- Sincerely Richard Schwab
participants (8)
-
Clément Junca
-
Cédric Girard
-
Dave Reisner
-
Doug Newgard
-
Jesse McClure
-
ponder＠creshal.de
-
Richard Schwab
-
Taylor Lookabaugh