Bug 147768

Summary: New Port: games/atanks 2D tank game
Product: Ports & Packages Reporter: Jesse <jessefrgsmith>
Component: Individual Port(s)Assignee: Matthias Andree <mandree>
Status: Closed FIXED    
Severity: Affects Only Me    
Priority: Normal    
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
file.shar none

Description Jesse 2010-06-10 18:20:01 UTC 
Please find attached a copy of the shar file for my new Port. This is a port of the simple arcade game Atomic Tanks (http://atanks.sf.net). The game relies on the Allegro graphics library.

This is my first attempt at making a Port. I've gone through the test steps listed in the Porters Handbook and everything seems to be working properly. However, as this is my first run through, I suspect there may be minor issues or questions of style to work out. Any feedback is appreciated.

Fix: Patch attached with submission follows:
Comment 1 Martin Wilke freebsd_committer freebsd_triage 2010-06-13 18:11:38 UTC 
Responsible Changed
From-To: freebsd-ports-bugs->miwi

I'll take it.
Comment 2 Martin Wilke freebsd_committer freebsd_triage 2010-07-12 06:37:10 UTC 
Responsible Changed
From-To: miwi->freebsd-port-bugs

Over to maintainer.
Comment 3 Martin Wilke freebsd_committer freebsd_triage 2010-07-12 07:14:03 UTC 
Responsible Changed
From-To: freebsd-port-bugs->freebsd-ports-bugs

back to pool
Comment 4 Matthias Andree freebsd_committer freebsd_triage 2010-08-12 16:07:28 UTC 
Responsible Changed
From-To: freebsd-ports-bugs->mandree

I'll take it.
Comment 5 Matthias Andree freebsd_committer freebsd_triage 2010-08-14 10:59:39 UTC 
State Changed
From-To: open->analyzed

work in progress
Comment 6 Matthias Andree 2010-08-14 12:31:19 UTC 
Hi Jesse,

there are a few remarks to make, but generally speaking, it is sound
work for a first-timer. Thanks a lot.

1. your patches are hard-coding /usr/local. This is frowned upon,
because users can override this with ${PREFIX} (for installation) or
${LOCALBASE}.

We usually use a post-extract or pre-patch or post-patch makefile target
and ${REINPLACE_CMD} to get this right.

Some of the patches you've made are dispensable, because INSTALLDIR is
inherited and properly derived from PREFIX - we only need to make sure
to add a trailing slash (I do that in MAKE_ENV).

2. Regarding the files, I've removed the patch for src/Makefile since
that inherits from Makefile, and moved two hunks from the Makefile patch
to a REINPLACE_CMD, and added a few more to clean things up a bit and,
for instance, strip the executable unless you install with WITH_DEBUG=yes.

3. -DMACOSX is bogus for FreeBSD and causes invalid C++ conversions;
however the upstream files.cpp code is broken. #include <sys/stat.h> (in
files.cpp) needs to happen unconditionally, on all Unices, including
Linux - it works by chance, not because it's proper code. Please forward
this as bug report or patch to the upstream.

I've added a patch under files/ for the time being. Please check if it's
still needed on the next atanks release.

I've also added an OSVERSION change to patch files.cpp in accordance
with the FreeBSD version, whether it uses the POSIX (IEEE Std 1003.1)
prototype for scandir (with const struct dirent* or non-const). This was
subtle and cost quite some time. Unfortunately we don't get away without
version check.

I've broken some very very early versions of FreeBSD 9-CURRENT; but I
don't care, people are supposed to update 9-CURRENT frequently.

4. style issues (minor):

> $ portlint -CN
> WARN: /usr/ports.cvs/games/atanks/pkg-plist: seems to have unnecessary
blank lines at the last part.

I've removed the blank line.

> WARN: Makefile: [1]: whitespace before end of line.
> WARN: Makefile: [5]: whitespace before end of line.

These cause another portlint artifact that goes away after removing the
blanks:

> FATAL: Makefile: no $FreeBSD$ line in comment section.

These are minor formatting issues:

> WARN: Makefile: COMMENT should begin with a capital, and end without a
period

=> I've removed the period

> WARN: Makefile: extra item placed in the *_DEPENDS section, for
example, "USE_GMAKE".

=> I've inserted a blank line.

5. I've added bin/atanks and @dirrmtry share/games to pkg-plist - "port
test" should have showed the former missing, that latter only appears in
tinderbox.

6. I've edited the MASTER_SITE because else the file couldn't be found.

Please review the resulting port. I am attaching what I've committed.

7. I've changed RUN_DEPENDS=allegro... to LIB_DEPENDS=alleg.42 because
the former was bogus, but perhaps through changes made to the allegro
port in the meanwhile (it's been a long time since you'd submitted the
port).

I'm currently test building on FreeBSD 6.4 in a tinderbox. If that goes
well, I will commit the new port.

Best regards
Matthias
Comment 7 dfilter service freebsd_committer freebsd_triage 2010-08-14 13:03:01 UTC 
mandree     2010-08-14 12:02:48 UTC

  FreeBSD ports repository

  Modified files:
    games                Makefile 
  Added files:
    games/atanks         Makefile distinfo pkg-descr pkg-plist 
    games/atanks/files   patch-Makefile patch-src_files.cpp 
  Log:
  Add new port games/atanks.
  
  Submitted by: Jesse Smith <jessefrgsmith@yahoo.ca>
  PR: ports/147768
  
  Revision  Changes    Path
  1.1341    +1 -0      ports/games/Makefile
  1.1       +38 -0     ports/games/atanks/Makefile (new)
  1.1       +3 -0      ports/games/atanks/distinfo (new)
  1.1       +11 -0     ports/games/atanks/files/patch-Makefile (new)
  1.1       +13 -0     ports/games/atanks/files/patch-src_files.cpp (new)
  1.1       +9 -0      ports/games/atanks/pkg-descr (new)
  1.1       +282 -0    ports/games/atanks/pkg-plist (new)
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org"
Comment 8 Matthias Andree freebsd_committer freebsd_triage 2010-08-14 13:03:04 UTC 
State Changed
From-To: analyzed->closed

New port added, with notable changes. Thanks!