Re: Pqiv Image Viewer
Claudio claudiozu...@gmail.com writes:
Is there a chance of getting this into ports if it's fine?
There was a bit of cleanup to do before thinking about that.
Here's another tarball, which fixes:
- ordering of variables (see Makefile.template)
- void CFLAGS and LDFLAGS; they were not passed to the build,
same for FAKE_FLAGS
- CFLAGS not respected, which leads to the build using -O3; use
MAKE_FLAGS
- dependency on gtk+3 (and not gtk+2)
- manpage formatting, mandoc doesn't support the .po roff request.
- DISTFILES handling using the less redundant distname{name}suffix
syntax.
- incomplete WANTLIB (+ gettext module)
- out of sync Makefile patch (unneeded hunks and comments)
- DESCR could give more information (is that enough?)
About the build warnings,
ok?
pqiv.tgz
Description: Binary data
--
jca | PGP: 0x06A11494 / 61DB D9A0 00A4 67CF 2A90 8961 6191 8FBF 06A1 1494
Re: Pqiv Image Viewer
On Wed, Dec 11, 2013 at 02:34:22PM +0100, Jérémie Courrèges-Anglas wrote:
Claudio claudiozu...@gmail.com writes:
Is there a chance of getting this into ports if it's fine?
There was a bit of cleanup to do before thinking about that.
Here's another tarball, which fixes:
- ordering of variables (see Makefile.template)
- void CFLAGS and LDFLAGS; they were not passed to the build,
same for FAKE_FLAGS
- CFLAGS not respected, which leads to the build using -O3; use
MAKE_FLAGS
- dependency on gtk+3 (and not gtk+2)
- manpage formatting, mandoc doesn't support the .po roff request.
- DISTFILES handling using the less redundant distname{name}suffix
syntax.
- incomplete WANTLIB (+ gettext module)
- out of sync Makefile patch (unneeded hunks and comments)
- DESCR could give more information (is that enough?)
No need to LIB_DEPENDS on cairo/glib2, gtk+3 already does, and use tabs
instead of spaces (COMMENT, USE/NO_* lines..)
With that, looks ok to me.
Landry
Re: Pqiv Image Viewer
Landry Breuil lan...@rhaalovely.net writes: [...] No need to LIB_DEPENDS on cairo/glib2, gtk+3 already does, Is No need to to be interpreted here as should not, ie. a general porting recommendation? Upstream explicitely uses this: `pkg-config --libs gtk+-3.0 glib-2.0 cairo gio-2.0` If for whatever reason our gtk+3 doesn't depend on cairo anymore the deps will be out of sync. And I see no problem with listing direct deps explicitely. This question is afaik not specific to gtk+3. and use tabs instead of spaces (COMMENT, USE/NO_* lines..) With that, looks ok to me. Landry -- jca | PGP: 0x06A11494 / 61DB D9A0 00A4 67CF 2A90 8961 6191 8FBF 06A1 1494
Re: Pqiv Image Viewer
On Wed, Dec 11, 2013 at 03:12:58PM +0100, Jérémie Courrèges-Anglas wrote: Landry Breuil lan...@rhaalovely.net writes: [...] No need to LIB_DEPENDS on cairo/glib2, gtk+3 already does, Is No need to to be interpreted here as should not, ie. a general porting recommendation? Upstream explicitely uses this: `pkg-config --libs gtk+-3.0 glib-2.0 cairo gio-2.0` If for whatever reason our gtk+3 doesn't depend on cairo anymore the deps will be out of sync. And I see no problem with listing direct deps explicitely. This question is afaik not specific to gtk+3. smaller is better, but that's a matter of taste :) Landry
Re: Pqiv Image Viewer
On Wed, Dec 11, 2013 at 03:22:20PM +0100, Landry Breuil wrote: On Wed, Dec 11, 2013 at 03:12:58PM +0100, Jérémie Courrèges-Anglas wrote: Landry Breuil lan...@rhaalovely.net writes: [...] No need to LIB_DEPENDS on cairo/glib2, gtk+3 already does, Is No need to to be interpreted here as should not, ie. a general porting recommendation? Upstream explicitely uses this: `pkg-config --libs gtk+-3.0 glib-2.0 cairo gio-2.0` If for whatever reason our gtk+3 doesn't depend on cairo anymore the deps will be out of sync. And I see no problem with listing direct deps explicitely. This question is afaik not specific to gtk+3. smaller is better, but that's a matter of taste :) Usually what _i_ do is only specifying direct deps if upstream insists on using a particular version - and that tends to bitrot over time if you forget to update it. Ie look at www/webkit's LIB_DEPENDS. But no one will yell at you for specifying gtk3/glib2/cairo, though in that particular case i really doubt gtk3 will stop using glib2 cairo someday. Landry
Re: Pqiv Image Viewer
On 2013/12/11 15:12, Jérémie Courrèges-Anglas wrote: Landry Breuil lan...@rhaalovely.net writes: [...] No need to LIB_DEPENDS on cairo/glib2, gtk+3 already does, Is No need to to be interpreted here as should not, ie. a general porting recommendation? Upstream explicitely uses this: `pkg-config --libs gtk+-3.0 glib-2.0 cairo gio-2.0` imho it would be correct to list the dependency in this case, though in practice it only really makes a difference on !shared arch, and gtk+3 is shared-only. If for whatever reason our gtk+3 doesn't depend on cairo anymore the deps will be out of sync. And I see no problem with listing direct deps explicitely. This question is afaik not specific to gtk+3. I'd kind-of prefer for them to be visibly out of sync (port doesn't build) as at least then there's a chance the WANTLIB will be regenerated ;)
Re: Pqiv Image Viewer
I'd kind-of prefer for them to be visibly out of sync (port doesn't build) as at least then there's a chance the WANTLIB will be regenerated ;) I personally never put the whole dep chain into LIB_DEPENDS -- the reason is that I work on the whole tree regularly and it is way easier if you don't have the explicit LIB_DEPENDS, that can save you a bump. e.g. when glib2,-main moved to glib2 I had to bump all ports that had a RUN+LIB_DEPENDS on devel/glib2 and I am *very* happy not every port that has glib-2.0 in WANTLIB also has a direct LIB_DEPENDS to devel/glib2, that would have been a bigger nightmare than it already was. -- Antoine
Re: Pqiv Image Viewer
Is there a chance of getting this into ports if it's fine?
Claudio
Claudio claudiozu...@gmail.com writes:
On Sun, Dec 08, 2013 at 03:07:33PM +0100, Jérémie Courrèges-Anglas wrote:
Claudio claudiozu...@gmail.com writes:
I opened that issue, the problem is 2.1 hasn't been release yet so it'd
be straight from git, would a do-install (as suggested by Stuart
Henderson) be better then ?
Sure.
Could you point me to a do-install example ?
do-install:
ls -l ${WRKSRC} ${PREFIX}
[...]
--
jca | PGP: 0x06A11494 / 61DB D9A0 00A4 67CF 2A90 8961 6191 8FBF 06A1 1494
Re: Pqiv Image Viewer
On 2013/12/08 14:09, Claudio wrote:
It's my first port, pqiv is a minimalist image viewer inspired by qiv.
Claudio
Not tried building it yet, but a few comments from reading:
COMMENT=very small and pretty fast gdk/Imlib image viewer
COMMENT=command line image viewer, replacement for qiv
two COMMENTs? also this is missing the rcs id line.
+MANDIR=$(PREFIX)/share/man
I'd just patch the install target to use ${PREFIX}/man/man1 rather
than add this indirection.
install: pqiv$(EXECUTABLE_EXTENSION)
- install -D pqiv$(EXECUTABLE_EXTENSION)
$(DESTDIR)$(PREFIX)/bin/pqiv$(EXECUTABLE_EXTENSION)
- install -D pqiv.1 $(DESTDIR)$(PREFIX)/share/man/man1/pqiv.1
+ mkdir -p $(DESTDIR)$(PREFIX)/bin
+ install pqiv$(EXECUTABLE_EXTENSION)
$(DESTDIR)$(PREFIX)/bin/pqiv$(EXECUTABLE_EXTENSION)
+ mkdir -p $(DESTDIR)$(MANDIR)/man1
+ install pqiv.1 $(DESTDIR)$(MANDIR)/man1/pqiv.1
no need for mkdir here, but this is missing handling for stripping.
so actually, rather than patch this at all, it's such a simple case that
it's probably easier to use a custom do-install target in the port Makefile.
uninstall:
rm -f $(DESTDIR)$(PREFIX)/bin/pqiv$(EXECUTABLE_EXTENSION)
- rm -f $(DESTDIR)$(PREFIX)/share/man/man1/pqiv.1
+ rm -f $(DESTDIR)$(MANDIR)/man1/pqiv.1
we won't run this, so no need to patch the uninstall target
Re: Pqiv Image Viewer
Stuart Henderson st...@openbsd.org writes:
On 2013/12/08 14:09, Claudio wrote:
It's my first port, pqiv is a minimalist image viewer inspired by qiv.
Claudio
Not tried building it yet, but a few comments from reading:
COMMENT=very small and pretty fast gdk/Imlib image viewer
COMMENT=command line image viewer, replacement for qiv
two COMMENTs? also this is missing the rcs id line.
I suggested the following changes so that Claudio could push them
upstream. The changes were accepted and should be in pqiv-2.1:
https://github.com/phillipberndt/pqiv/issues/17
If 2.0 has to be imported then I guess a lighter patch could be used.
+MANDIR=$(PREFIX)/share/man
I'd just patch the install target to use ${PREFIX}/man/man1 rather
than add this indirection.
install: pqiv$(EXECUTABLE_EXTENSION)
- install -D pqiv$(EXECUTABLE_EXTENSION)
$(DESTDIR)$(PREFIX)/bin/pqiv$(EXECUTABLE_EXTENSION)
- install -D pqiv.1 $(DESTDIR)$(PREFIX)/share/man/man1/pqiv.1
+ mkdir -p $(DESTDIR)$(PREFIX)/bin
+ install pqiv$(EXECUTABLE_EXTENSION)
$(DESTDIR)$(PREFIX)/bin/pqiv$(EXECUTABLE_EXTENSION)
+ mkdir -p $(DESTDIR)$(MANDIR)/man1
+ install pqiv.1 $(DESTDIR)$(MANDIR)/man1/pqiv.1
no need for mkdir here, but this is missing handling for stripping.
so actually, rather than patch this at all, it's such a simple case that
it's probably easier to use a custom do-install target in the port Makefile.
uninstall:
rm -f $(DESTDIR)$(PREFIX)/bin/pqiv$(EXECUTABLE_EXTENSION)
- rm -f $(DESTDIR)$(PREFIX)/share/man/man1/pqiv.1
+ rm -f $(DESTDIR)$(MANDIR)/man1/pqiv.1
we won't run this, so no need to patch the uninstall target
--
jca | PGP: 0x06A11494 / 61DB D9A0 00A4 67CF 2A90 8961 6191 8FBF 06A1 1494
Re: Pqiv Image Viewer
I opened that issue, the problem is 2.1 hasn't been release yet so it'd be
straight from git, would a do-install (as suggested by Stuart Henderson) be
better then ? Could you point me to a do-install example ?
Thank you
Claudio
On Sun, Dec 08, 2013 at 02:31:01PM +0100, Jérémie Courrèges-Anglas wrote:
Stuart Henderson st...@openbsd.org writes:
On 2013/12/08 14:09, Claudio wrote:
It's my first port, pqiv is a minimalist image viewer inspired by qiv.
Claudio
Not tried building it yet, but a few comments from reading:
COMMENT=very small and pretty fast gdk/Imlib image viewer
COMMENT=command line image viewer, replacement for qiv
two COMMENTs? also this is missing the rcs id line.
I suggested the following changes so that Claudio could push them
upstream. The changes were accepted and should be in pqiv-2.1:
https://github.com/phillipberndt/pqiv/issues/17
If 2.0 has to be imported then I guess a lighter patch could be used.
+MANDIR=$(PREFIX)/share/man
I'd just patch the install target to use ${PREFIX}/man/man1 rather
than add this indirection.
install: pqiv$(EXECUTABLE_EXTENSION)
- install -D pqiv$(EXECUTABLE_EXTENSION)
$(DESTDIR)$(PREFIX)/bin/pqiv$(EXECUTABLE_EXTENSION)
- install -D pqiv.1 $(DESTDIR)$(PREFIX)/share/man/man1/pqiv.1
+ mkdir -p $(DESTDIR)$(PREFIX)/bin
+ install pqiv$(EXECUTABLE_EXTENSION)
$(DESTDIR)$(PREFIX)/bin/pqiv$(EXECUTABLE_EXTENSION)
+ mkdir -p $(DESTDIR)$(MANDIR)/man1
+ install pqiv.1 $(DESTDIR)$(MANDIR)/man1/pqiv.1
no need for mkdir here, but this is missing handling for stripping.
so actually, rather than patch this at all, it's such a simple case that
it's probably easier to use a custom do-install target in the port Makefile.
uninstall:
rm -f $(DESTDIR)$(PREFIX)/bin/pqiv$(EXECUTABLE_EXTENSION)
- rm -f $(DESTDIR)$(PREFIX)/share/man/man1/pqiv.1
+ rm -f $(DESTDIR)$(MANDIR)/man1/pqiv.1
we won't run this, so no need to patch the uninstall target
--
jca | PGP: 0x06A11494 / 61DB D9A0 00A4 67CF 2A90 8961 6191 8FBF 06A1 1494
Re: Pqiv Image Viewer
Claudio claudiozu...@gmail.com writes:
I opened that issue, the problem is 2.1 hasn't been release yet so it'd
be straight from git, would a do-install (as suggested by Stuart
Henderson) be better then ?
Sure.
Could you point me to a do-install example ?
do-install:
ls -l ${WRKSRC} ${PREFIX}
[...]
--
jca | PGP: 0x06A11494 / 61DB D9A0 00A4 67CF 2A90 8961 6191 8FBF 06A1 1494
Re: Pqiv Image Viewer
Is this better?
Claudio
On Sun, Dec 08, 2013 at 03:07:33PM +0100, Jérémie Courrèges-Anglas wrote:
Claudio claudiozu...@gmail.com writes:
I opened that issue, the problem is 2.1 hasn't been release yet so it'd
be straight from git, would a do-install (as suggested by Stuart
Henderson) be better then ?
Sure.
Could you point me to a do-install example ?
do-install:
ls -l ${WRKSRC} ${PREFIX}
[...]
--
jca | PGP: 0x06A11494 / 61DB D9A0 00A4 67CF 2A90 8961 6191 8FBF 06A1 1494
pqiv.tar.gz
Description: application/tar-gz
Re: Pqiv Image Viewer
Forgot to remove the MANDIR , it's commented out.
Claudio
On Sun, Dec 08, 2013 at 03:07:33PM +0100, Jérémie Courrèges-Anglas wrote:
Claudio claudiozu...@gmail.com writes:
I opened that issue, the problem is 2.1 hasn't been release yet so it'd
be straight from git, would a do-install (as suggested by Stuart
Henderson) be better then ?
Sure.
Could you point me to a do-install example ?
do-install:
ls -l ${WRKSRC} ${PREFIX}
[...]
--
jca | PGP: 0x06A11494 / 61DB D9A0 00A4 67CF 2A90 8961 6191 8FBF 06A1 1494
