[aur-general] PKGBUILD review request - Aur-general - lists.archlinux.org

newer
[aur-general] Signoff report for...

[aur-general] PKGBUILD review request

older
[aur-general] Removal request of...

Douglas Christman

10 Jan 2014 10 Jan '14

11:59 p.m.

Hello, This is my first attempt at creating a package so I'd appreciate any feedback you have. The PKGBUILD is avilable here: https://raw.github.com/dobyrch/termboy-go/master/PKGBUILD Thanks!

Show replies by date

Jason St. John

11 Jan 11 Jan

12:32 a.m.

On Fri, Jan 10, 2014 at 6:59 PM, Douglas Christman wrote:

Hello,

This is my first attempt at creating a package so I'd appreciate any feedback you have. The PKGBUILD is avilable here: https://raw.github.com/dobyrch/termboy-go/master/PKGBUILD

Thanks!

Hello, You should quote the link in the source array and any string that includes a Bash variable (e.g. use `install -Dm 755 "$pkgname-go-$pkgver" "$pkgdir/usr/bin/$pkgname-go"`). You can remove the dependency on 'bash' because you shouldn't include dependency information for any packages in the groups 'base' or 'base-devel'. Unless this package will _only_ work for 64-bit systems, you should change the arch array to the following: arch=('i686' 'x86_64') And this is a minor issue, but you should change the GitHub links to use https:// so the source download will _always_ use HTTPS, even if the http -> https redirect that GitHub uses stops working for whatever reason. Jason

Isaac Dupree

1:03 a.m.

On 01/10/2014 07:32 PM, Jason St. John wrote:

Unless this package will _only_ work for 64-bit systems, you should change the arch array to the following: arch=('i686' 'x86_64')

Should the documentation be updated? [1] describes "arch" as "An array of architectures that the PKGBUILD file is known to build and work on." That description implies to me that if you haven't tested it on i686 then you shouldn't list i686. However, the actual AUR convention is, as you say, to include i686 and x86_64 unless you know it's nonportable. [1] https://wiki.archlinux.org/index.php/PKGBUILD#arch

Anatol Pomozov

1:10 a.m.

Hi On Fri, Jan 10, 2014 at 4:32 PM, Jason St. John wrote:

You should quote the link in the source array and any string that includes a Bash variable (e.g. use `install -Dm 755 "$pkgname-go-$pkgver" "$pkgdir/usr/bin/$pkgname-go"`).

Could you please please explain it? Why bash variables need quotes? Well, $srcdir/$destdir need double-quotes because they might contain spaces (although creating dirs with white-spaces on UNIX is a dumb idea). What about other variables like $pkgver? The same question applied to other parts of PKGBUILD - why quotes are needed for dependencies, license, arch, sources...?

Sam Stuewe

1:23 a.m.

On 2014-01-10 19:10, Anatol Pomozov wrote:

Well, $srcdir/$destdir need double-quotes because they might contain spaces (although creating dirs with white-spaces on UNIX is a dumb idea). What about other variables like $pkgver? The same question applied to other parts of PKGBUILD - why quotes are needed for dependencies, license, arch, sources...?

It's not actually required if you know that the variable won't expand incorrectly. However, it is good practice to do so. And, using double-quotes will ensure proper expansion. I tend to be incredibly verbose about this and even include braces for variables: "${pkgname}-${pkgver}.tar.gz" It looks cleanest to me this way, and I know that it will always expand the way I want it to. This is, however, a matter of style more than anything else. -- All the best, Sam Stuewe (HalosGhost)

Evgeniy Alekseev

12:36 a.m.

Hi! On Friday 10 January 2014 18:59:35 Douglas Christman wrote:

This is my first attempt at creating a package so I'd appreciate any feedback you have. The PKGBUILD is avilable here: https://raw.github.com/dobyrch/termboy-go/master/PKGBUILD

--------------- it was written by Jason --------------- 1. It is highly recommended using double quotes for paths (to avoid problems with paths with spaces, for example). 2. What about i686 architecture? If it is supported it must be added to arch list. 3. Dependency 'bash' could be removed because it is in base group and several packages in base-devel group depend on it [1, see Warning message]. --------------- it was written by Jason --------------- 4. It has several errors during build (could not find packages github.com/dobyrch/*). I think you must fix it in the upstream =) [1]. https://wiki.archlinux.org/index.php/AUR#Prerequisites -- С уважением, Е.Алексеев. Sincerely yours, E.Alekseev. e-mail: darkarcanis@mail.ru ICQ: 407-398-235 Jabber: arcanis@jabber.ru

Douglas Christman

1:35 a.m.

It looks like GOPATH needed to be set for it to build correctly. Would you mind testing out the changes I made to see if it fixes the problem? https://raw.github.com/dobyrch/termboy-go/master/PKGBUILD On Fri, Jan 10, 2014 at 7:36 PM, Evgeniy Alekseev wrote:

Hi!

On Friday 10 January 2014 18:59:35 Douglas Christman wrote:

...
This is my first attempt at creating a package so I'd appreciate any feedback you have. The PKGBUILD is avilable here: https://raw.github.com/dobyrch/termboy-go/master/PKGBUILD

--------------- it was written by Jason --------------- 1. It is highly recommended using double quotes for paths (to avoid problems with paths with spaces, for example). 2. What about i686 architecture? If it is supported it must be added to arch list. 3. Dependency 'bash' could be removed because it is in base group and several packages in base-devel group depend on it [1, see Warning message]. --------------- it was written by Jason ---------------

4. It has several errors during build (could not find packages github.com/dobyrch/*). I think you must fix it in the upstream =)

[1]. https://wiki.archlinux.org/index.php/AUR#Prerequisites -- С уважением, Е.Алексеев. Sincerely yours, E.Alekseev.

e-mail: darkarcanis@mail.ru ICQ: 407-398-235 Jabber: arcanis@jabber.ru

Evgeniy Alekseev

1:55 a.m.

On Friday 10 January 2014 20:35:45 Douglas Christman wrote:

It looks like GOPATH needed to be set for it to build correctly. Would you mind testing out the changes I made to see if it fixes the problem?

https://raw.github.com/dobyrch/termboy-go/master/PKGBUILD

On Fri, Jan 10, 2014 at 7:36 PM, Evgeniy Alekseev wrote:

...
4. It has several errors during build (could not find packages github.com/dobyrch/*). I think you must fix it in the upstream =)

Yes, now it builds normally. But anyway I think that patching of source files in the upstream (or creating a patch that does this in the prepare() function) will be better way than moving of sources to the correct path. But as you wish. And it is recommended using a patching/moving/etc in the prepare() function not build() or package(). BTW instead "src/github.com/dobyrch" will be better to use "$srcdir" variable (e.g. "$srcdir/github.com/dobyrch"). -- С уважением, Е.Алексеев. Sincerely yours, E.Alekseev. e-mail: darkarcanis@mail.ru ICQ: 407-398-235 Jabber: arcanis@jabber.ru

Jeremy Audet

6:28 a.m.

I created a pull request on GitHub. See: https://github.com/dobyrch/termboy-go/pull/1 --Jeremy

3789

Age (days ago)

3790

Last active (days ago)

Download

8 comments

7 participants

tags

participants (7)

Anatol Pomozov
Douglas Christman
Evgeniy Alekseev
Isaac Dupree
Jason St. John
Jeremy Audet
Sam Stuewe