Opened 3 months ago

Last modified 3 months ago

#69342 new submission

shaderc: new port

Reported by: DanielO (Daniel O'Connor) Owned by:
Priority: Normal Milestone:
Component: ports Version:
Keywords: haspatch Cc:
Port: shaderc

Description

Add shaderc port.

Attachments (2)

shaderc-newport.diff (2.9 KB) - added by DanielO (Daniel O'Connor) 3 months ago.
shaderc-newport2.diff (3.1 KB) - added by DanielO (Daniel O'Connor) 3 months ago.
Updated port diff.

Download all attachments as: .zip

Change History (4)

Changed 3 months ago by DanielO (Daniel O'Connor)

Attachment: shaderc-newport.diff added

comment:1 Changed 3 months ago by ryandesign (Ryan Carsten Schmidt)

Thanks!

Is there a reason why you used the old cmake 1.0 portgroup instead of the current cmake 1.1 portgroup?

There is no need to set name or homepage because github.setup sets them for you.

Email addresses in the maintainers line should not be written in literal user@host format but rather in obfuscated host:user format; MacPorts will decode these when it displays them.

Email addresses in maintainers should be accompanied by GitHub handles, as in the other two ports you maintain: {dons.net.au:darius @DanielO}.

Unless there is a good reason to prohibit it, we prefer to have permission up-front to make minor modifications to the port without consulting the maintainer. This permission can be indicated by adding openmaintainer to the maintainers line. This applies to the two existing ports you maintain as well.

Your long description will not appear properly when displayed by MacPorts because MacPorts combines all lines into one. To insert newlines that will be preserved, use \n. For an example, see the mongo-tools Portfile.

The glslang and spirv-tools ports both install dynamic libraries but you've declared them as depends_build. Are you certain that shaderc does not use those dynamic libraries? If it does use them, then they must be depends_lib instead.

The trailing backslash on the -DSHADERC_SKIP_TESTS=yes \ line should be removed.

-I flags typically go in configure.cppflags not configure.cxxflags, and there is no reason to use quotation marks around them.

In your patchfile, it would be shorter, simpler, and easier to read if you removed the add_subdirectory(third_party) line rather than commenting it out.

Could you add sentence or two at the top of the patchfile explaining what it is doing and providing any applicable URLs (bug reports, pull requests, etc.)?

The README says it requires C++17. This should be indicated with the line compiler.cxx_standard 2017 so that MacPorts can pick a suitable compiler.

It also says it needs python 3 for utility scripts (and tests but you're skipping the tests). If those utility scripts are run as part of the build and/or get installed, then you'll need a python 3 dependency and to tell the software to use that version of python 3. Simplest would be to use port:python312 unconditionally. More complicated could be to use the macOS version of python 3 on those versions of macOS that have it. The magic Portfile gives an example of the more complicated strategy, but note that the way that you communicate the desired python choice will be specific to shaderc. It may involve cmake flags, environment variables, or even patchfiles.

comment:2 Changed 3 months ago by DanielO (Daniel O'Connor)

Hi, I used the old PortGroup because I cargoculted the portfile from vulkan-loader. I think I've fixed all the other points you raised, thank you. There are no Python scripts installed, I believe they are only used for tests.

Changed 3 months ago by DanielO (Daniel O'Connor)

Attachment: shaderc-newport2.diff added

Updated port diff.

Note: See TracTickets for help on using tickets.