Opened 10 years ago

Closed 10 years ago

#43579 closed submission (fixed)

zathura-plugin-pdf-poppler @0.2.5

Reported by: harciga Owned by: macports-tickets@…
Priority: Normal Milestone:
Component: ports Version: 2.2.1
Keywords: Cc:
Port: zathura-plugin-pdf-poppler

Description

new port

Attachments (10)

Portfile (1.4 KB) - added by harciga 10 years ago.
patch-Makefile (2.1 KB) - added by harciga 10 years ago.
patch-config.mk (315 bytes) - added by harciga 10 years ago.
Portfile.2 (1.4 KB) - added by harciga 10 years ago.
patch-Makefile.diff (2.1 KB) - added by harciga 10 years ago.
patch-config.mk.diff (315 bytes) - added by harciga 10 years ago.
Portfile.3 (1.4 KB) - added by harciga 10 years ago.
Portfile.4 (1.4 KB) - added by harciga 10 years ago.
Portfile.5 (1.6 KB) - added by harciga 10 years ago.
Portfile.6 (1.6 KB) - added by harciga 10 years ago.

Download all attachments as: .zip

Change History (21)

Changed 10 years ago by harciga

Attachment: Portfile added

Changed 10 years ago by harciga

Attachment: patch-Makefile added

Changed 10 years ago by harciga

Attachment: patch-config.mk added

comment:1 Changed 10 years ago by harciga

It's actually version 0.2.5, the latest. Wouldn't let me modify it.

comment:2 Changed 10 years ago by ryandesign (Ryan Carsten Schmidt)

Thanks. Some observations about the Portfile:

  • There should be a blank line between the "# $Id$" line and the "PortSystem 1.0" line; running "port lint --nitpick" should mention that.
  • The license field should be just "zlib", not "zlib License" (the license field is a space-separated list of values; consult the port_binary_distributable.tcl script for a non-exhaustive list of license names)
  • Checksums are not used for patchfiles that will be copied to the files directory; they are used for patchfiles that get downloaded, but we only do that in the very unusual circumstance that a patchfile is provided by a third party and it is too large to be conveniently copied into the files directory.
  • The checksums line doesn't need to mention the distfile name, because there is only one file being checksummed.
  • Patchfile names should be of the form "patch-*.diff", as "port lint --nitpick" should mention.
  • Because use you "use_configure no", you must add code to ensure you're UsingTheRightCompiler, build with -arch flags, and if possible add a universal variant.
  • lib- and bin-style dependencies allow non-MacPorts software to satisfy them. We don't normally want that; see wiki:FAQ. So usually you should use port-style dependencies (if only a single port exists that can satisfy it), or path-style dependencies (if there are multiple ports that can satisfy it). Or if there is another reason why you used lib- and bin-style dependencies in this port, let me know.

Regarding patch-Makefile:

  • The install_name of a library should be the final location of the library when it is installed; it should not begin with ${DESTDIR}
  • The install_name should usually be set using the "-install_name" flag; is there a particular reason you used "-Wl,-dylib_install_name," instead?

comment:3 Changed 10 years ago by ryandesign (Ryan Carsten Schmidt)

Keywords: pwmt removed
Summary: zathura-plugin-pdf-poppler @0.2.7zathura-plugin-pdf-poppler @0.2.5

comment:4 in reply to:  2 Changed 10 years ago by harciga

Replying to ryandesign@…:

Thanks. Some observations about the Portfile:

I'll address these issues tomorrow, as well as in the other 2 dependant port submissions (zathura & girara)

comment:5 in reply to:  2 Changed 10 years ago by harciga

Replying to ryandesign@…:

Thanks. Some observations about the Portfile:

  • There should be a blank line between the "# $Id$" line and the "PortSystem 1.0" line; running "port lint --nitpick" should mention that.

Yup, fixed the warnings. port lint is mentioned at §3.1.18 lint https://guide.macports.org/index.html#using.port.lint, I missed it. Perhaps it should be reminded of at §4.7 Portfile Best Practices?

  • The license field should be just "zlib", not "zlib License" (the license field is a space-separated list of values; consult the port_binary_distributable.tcl script for a non-exhaustive list of license names)

Fixed, the guide at §5.1 Global keywords https://guide.macports.org/index.html#reference.keywords makes no mention of this, should I open a ticket?

  • Checksums are not used for patchfiles that will be copied to the files directory; they are used for patchfiles that get downloaded, but we only do that in the very unusual circumstance that a patchfile is provided by a third party and it is too large to be conveniently copied into the files directory.

Removed. port distfiles PORTNAME gives reason to think patchfiles are distfiles. From §5.3.4 Checksum Phase Keywords

At least two checksum types (e.g., rmd160 and sha256) should be used to ensure the integrity of the distfiles.

  • The checksums line doesn't need to mention the distfile name, because there is only one file being checksummed.

Ok.

  • Patchfile names should be of the form "patch-*.diff", as "port lint --nitpick" should mention.

Fixed.

  • Because use you "use_configure no", you must add code to ensure you're UsingTheRightCompiler, build with -arch flags, and if possible add a universal variant.

-- UsingTheRightCompiler

I have checked the Makefile and it uses ${CC}, I also added the recommended lines for Ports with nonstandard or non-existent configure scripts https://trac.macports.org/wiki/UsingTheRightCompiler#nonstandard-ports

-- build with -arch- flags

Is this not handled by default?

-- add a universal variant

Why? §5.3.7.1 Configure Universal

There is a default universal variant made available to all ports by MacPorts base, so redefining universal keywords should only be done to make a given port compile if the default options fail to do so.

  • lib- and bin-style dependencies allow non-MacPorts software to satisfy them. We don't normally want that; see wiki:FAQ. So usually you should use port-style dependencies (if only a single port exists that can satisfy it), or path-style dependencies (if there are multiple ports that can satisfy it). Or if there is another reason why you used lib- and bin-style dependencies in this port, let me know.

Switched both lib: and bin: dependencies to path: style. OS X is not supported by pwnt.org nor alternative package managers, so only macports alternative -devel ports will satisfy the dependencies in the foreseeable future.

Regarding patch-Makefile:

  • The install_name of a library should be the final location of the library when it is installed; it should not begin with ${DESTDIR}

It has nothing to do with §7.2.4 DESTDIR: Support for Staged Installs http://www.gnu.org/prep/standards/html_node/DESTDIR.html It's the smallest change I could think of to fix the dynamic library lookup path from relative to absolute. It's a nitpick, it works correctly either way, but since I'm fixing port lint trailing whitespaces...

  • The install_name should usually be set using the "-install_name" flag; is there a particular reason you used "-Wl,-dylib_install_name," instead?

man ld line 323-324.

This option is also called -dylib_install_name for compatibility.

Changed 10 years ago by harciga

Attachment: Portfile.2 added

Changed 10 years ago by harciga

Attachment: patch-Makefile.diff added

Changed 10 years ago by harciga

Attachment: patch-config.mk.diff added

comment:6 Changed 10 years ago by harciga

Amended a path style dependency, it works either way so I guess there's some sort of sorcery going on in there.

Changed 10 years ago by harciga

Attachment: Portfile.3 added

Changed 10 years ago by harciga

Attachment: Portfile.4 added

comment:7 Changed 10 years ago by harciga

I had missed ${destroot} in destroot.args

Last edited 10 years ago by harciga (previous) (diff)

comment:8 Changed 10 years ago by harciga

Ok, added -arch flags and variant universal and tested the Portfile to work correctly.

Is there anything else I should address for approval?

Changed 10 years ago by harciga

Attachment: Portfile.5 added

comment:9 Changed 10 years ago by harciga

This Portfile is not ready for submission, variant universal is not building fat binaries and -arch flags while it builds cleanly produces a misbehaving binary.

I understand variant universal can be removed and worked at a later date if fat binaries are requested, but -arch flags are a requisite for port approval because of the use_configure no directive. I have to find out what is causing the bug.

Changed 10 years ago by harciga

Attachment: Portfile.6 added

comment:10 in reply to:  9 Changed 10 years ago by harciga

universal variant is working correctly now. And the application is behaving correctly.

$ lipo -info /opt/local/lib/zathura/pdf.dylib

Architectures in the fat file: /opt/local/lib/zathura/pdf.dylib are: x86_64 i386

Let me know if there is anything left to address.

comment:11 Changed 10 years ago by neverpanic (Clemens Lang)

Resolution: fixed
Status: newclosed

Committed in r120312 with the following changes:

  • Added missing build dependency on port:pkgconfig.
  • Added VERBOSE=1 to build.env to simplify debugging build failures in the future.
  • Modified patch-Makefile.diff to link as a bundle (-bundle) and specify the bundle loader (-Wl,-bundle_loader,${PREFIX}/bin/zathura) to avoid undefined symbols. Removed library version numbers because those aren't supported by ld64 in bundle mode.
Note: See TracTickets for help on using tickets.