par mangles UTF-8 input with a non-breaking space character U+00A0

[kenh@…]

Par isn't aware of UTF-8. Normally it works fine, but I've run into one particular problem. If you try to reflow UTF-8 text with a non-breaking space (U+00A0, encoded in UTF-8 as 0xC2 0xA0), the isspace() function returns true on the 0xA0, and par replaces that with a space character (0x20). That messes up the UTF-8 and makes other utilities unhappy.

The following patch (from the FreeBSD ports repository, not by me) solves this by checking to see if it's also an ascii character before treating it as a space. Solves the problem for me.

[kenh@…]

Patch for par

[qbarnes (Quentin Barnes)]

It's been awhile since I've made Macports changes, so please review my patches before merging them. I've attached my patch for par's Portfile and a tar file containing the new patchfiles.

Would someone review my changes and commit them if they're acceptable?

If there is any feedback on the patches, please send me email or make them here.

[qbarnes (Quentin Barnes)]

Oh, I've also contacted the maintainer of the Par utility to see if he'll merge the patches upstream. I haven't heard back from him yet.

[kenh@…]

Replying to qbarnes@…:

Would someone review my changes and commit them if they're acceptable?

I don't think I have commit privs, but I did test out your patches on a local copy of the par Portfile and they work great! So I think they can be committed as-is.

[ryandesign (Ryan Carsten Schmidt)]

I'm attaching a revised proposal. Let me know if this is ok to commit.

I added changes to ensure the port is UsingTheRightCompiler and -arch flags and added a universal variant. I also added our standard optimization flags.

I removed your patch to the protoMakefile because although it's nice to support CFLAGS, and hopefully the developer will incorporate that patch, the portfile doesn't actually set CFLAGS to anything, so the patch is pointless in this context.

[kenh@…]

Replying to ryandesign@…:

I'm attaching a revised proposal. Let me know if this is ok to commit.

FWIW, I tried out par with the new Portfile and it worked fine. So in terms of functionality I think it's ready to go.

Nothing against Adam Costello, the original author of par, but the last time it was updated was April 29th ... of 2001, 11 years ago. Holy balls, suddenly I feel old.

[qbarnes (Quentin Barnes)]

Ryan, did you look at the lines I deleted from the Portfile, specifically the "reinplace" line?

What I did was move the functionality of the reinplace line into a protoMakefile patch file.

Adding CFLAGS to the makefile dates back to 2004 when I first added the package. One of the maintainers back then required the addition. (I looked for the mail and couldn't find it.) Is having makefiles be able to pick up CFLAGS from the environment no longer necessary or desired?

The maintainer back then helped me come up with the reinplace line so I could avoid having to make a patch file for the makefile. Since I already now had other patch files, I no longer saw its benefit and just moved the change to the patch. If you notice, for the FreeBSD changes to par (​http://www.freebsd.org/cgi/cvsweb.cgi/ports/textproc/par/files/), they also modify the makefile to pick up CFLAGS. Since darwinports was based on BSD's ports, is it an old requirement?

[ryandesign (Ryan Carsten Schmidt)]

Replying to qbarnes@…:

Ryan, did you look at the lines I deleted from the Portfile, specifically the "reinplace" line?

What I did was move the functionality of the reinplace line into a protoMakefile patch file.

I agree that the CFLAGS lines of the new patchfile are functionally equivalent to the old reinplace. I just maintain that neither are necessary.

Adding CFLAGS to the makefile dates back to 2004 when I first added the package. One of the maintainers back then required the addition. (I looked for the mail and couldn't find it.) Is having makefiles be able to pick up CFLAGS from the environment no longer necessary or desired?

Having Makefiles be able to support CFLAGS, CPPFLAGS, LDFLAGS, CC, CXX etc variables is greatly desirable, and if this port were still being developed by its developer I would recommend you send the patch to them. But this port does not use the CFLAGS, CPPFLAGS, LDFLAGS variables so there's no point in doing this within MacPorts. MacPorts itself only supplies these variables when it runs a standard configure script, which this port does not have. We might be interested in manually supplying the CFLAGS, CPPFLAGS, LDFLAGS variables to the Makefile if par required that to help it link with other software, but par doesn't appear to link with any other software so that's not necessary.

The maintainer back then helped me come up with the reinplace line so I could avoid having to make a patch file for the makefile. Since I already now had other patch files, I no longer saw its benefit and just moved the change to the patch.

Indeed, when portfile variables are not involved, a patchfile is greatly preferable to a reinplace.

If you notice, for the FreeBSD changes to par (​http://www.freebsd.org/cgi/cvsweb.cgi/ports/textproc/par/files/), they also modify the makefile to pick up CFLAGS. Since darwinports was based on BSD's ports, is it an old requirement?

MacPorts was inspired by FreeBSD ports, but is written in a different language than FreeBSD ports and doesn't share any code with FreeBSD ports. I am not familiar with how FreeBSD ports works or how it uses the CFLAGS etc. variables. Presumably FreeBSD ports actually sets the CFLAGS variable to something at build time, and it wants the par port's Makefile to pick that up.

[qbarnes (Quentin Barnes)]

Replying to ryandesign@…:

Replying to qbarnes@…:

Ryan, did you look at the lines I deleted from the Portfile, specifically the "reinplace" line?

What I did was move the functionality of the reinplace line into a protoMakefile patch file.

I agree that the CFLAGS lines of the new patchfile are functionally equivalent to the old reinplace. I just maintain that neither are necessary.

Ah, if that's the case that it doesn't matter, you can discard the protoMakefile patch if you like.

I see you've tweaked the build.args and added variant universal too. Good to see. I'll remember those changes for other packages.

I'm not sure I completely follow your build.arg changes to support universal builds. I can't find the "[get_canonical_archflags cc]" documented. Is there a reason to use that Tcl escape hatch function instead of just "${configure.universal_cflags}"?

(I really have no idea. My entire understanding is from reading your changes and reading the macports doc page and making some extrapolations and guesswork.)

Adding CFLAGS to the makefile dates back to 2004 when I first added the package. One of the maintainers back then required the addition. (I looked for the mail and couldn't find it.) Is having makefiles be able to pick up CFLAGS from the environment no longer necessary or desired?

Having Makefiles be able to support CFLAGS, CPPFLAGS, LDFLAGS, CC, CXX etc variables is greatly desirable, and if this port were still being developed by its developer I would recommend you send the patch to them. But this port does not use the CFLAGS, CPPFLAGS, LDFLAGS variables so there's no point in doing this within MacPorts. MacPorts itself only supplies these variables when it runs a standard configure script, which this port does not have. We might be interested in manually supplying the CFLAGS, CPPFLAGS, LDFLAGS variables to the Makefile if par required that to help it link with other software, but par doesn't appear to link with any other software so that's not necessary.

Interesting. I just wish I had the original mails from back then to understand what was going through my head then and who I was talking with.

The maintainer back then helped me come up with the reinplace line so I could avoid having to make a patch file for the makefile. Since I already now had other patch files, I no longer saw its benefit and just moved the change to the patch.

Indeed, when portfile variables are not involved, a patchfile is greatly preferable to a reinplace.

If you notice, for the FreeBSD changes to par (​http://www.freebsd.org/cgi/cvsweb.cgi/ports/textproc/par/files/), they also modify the makefile to pick up CFLAGS. Since darwinports was based on BSD's ports, is it an old requirement?

MacPorts was inspired by FreeBSD ports, but is written in a different language than FreeBSD ports and doesn't share any code with FreeBSD ports. I am not familiar with how FreeBSD ports works or how it uses the CFLAGS etc. variables. Presumably FreeBSD ports actually sets the CFLAGS variable to something at build time, and it wants the par port's Makefile to pick that up.

I didn't realize the original darwinparts was a from scratch coding effort from the start. I had always thought it was a fork of FreeBSD's ports source and then rewritten and refactored over time.

[ryandesign (Ryan Carsten Schmidt)]

Replying to qbarnes@…:

Ah, if that's the case that it doesn't matter, you can discard the protoMakefile patch if you like.

I see you've tweaked the build.args and added variant universal too. Good to see. I'll remember those changes for other packages.

Only needed for ports that don't use a standard configure script, which would have handled it automatically.

I'm not sure I completely follow your build.arg changes to support universal builds. I can't find the "[get_canonical_archflags cc]" documented. Is there a reason to use that Tcl escape hatch function instead of just "${configure.universal_cflags}"?

Sorry, [get_canonical_archflags] isn't documented anywhere, as far as I know. It returns ${configure.universal_*flags} when the universal variant is set, and ${configure.*_archflags} when it isn't. Always using ${configure.universal_*flags} wouldn't be correct, as that would result in a universal build, even when the universal variant isn't selected.

I committed my patch in r92320.