lha: fix implicit function declarations

[chrstphrchvz]

Fixes: https://trac.macports.org/ticket/61535

Description

Not sure whether this is the best way to accommodate private API usage.

Tested on

macOS 10.15.7
Xcode 12.2 command line tools

Verification

Have you

• followed our Commit Message Guidelines?
• squashed and minimized your commits?
• checked that there aren't other open pull requests for the same change?
• referenced existing tickets on Trac with full URL?
• checked your Portfile with port lint?
• tried existing tests with sudo port test?
• tried a full install with sudo port -vst install?
• tested basic functionality of all binary files?

[ryandesign]

How about updating the port to a newer upstream commit instead?

In this commit they changed it so that it only tries to use private macOS functions if iconv is not available, which the port already declares a dependency on.

[macportsbot]

Travis Build #15315 Passed.

Lint results

--->  Verifying Portfile for lha
--->  0 errors and 0 warnings found.

Port lha success on xcode10.3. Log
Port lha success on xcode9.4. Log
Port lha success on xcode8.3. Log

[chrstphrchvz]

How about updating the port to a newer upstream commit instead?

In this commit they changed it so that it only tries to use private macOS functions if iconv is not available, which the port already declares a dependency on.

The port was already updated to the latest upstream commit, which descends from the one you linked to. Could something not be configuring as expected?

[ryandesign]

Oh. I assumed based on the port version "1.14i-ac20081023" that it was a version from October 23, 2008. But indeed it already uses the latest commit which is from October 4, 2019 and does include the fix I mentioned.

It should only go down this code path if HAVE_ICONV is not defined. And on my system indeed HAVE_ICONV is not defined, although HAVE_ICONV_H is defined.

There are no other occurrences of HAVE_ICONV in the lha source code, other than checking in this file whether it is defined. There are no occurrences of HAVE_ICONV_H in the lha source code. The lha source code does not include a generated configure script; we generate it with autoreconf. Maybe this is one of those things were an older version of autoconf used to set HAVE_ICONV but now sets HAVE_ICONV_H?

Using a version of MacPorts base containing the fix proposed in macports/macports-base#217 I see these additional messages after the configure phase:

Warning: Configuration logfile /opt/local/var/macports/build/_Users_rschmidt_macports_macports-ports-svn-trunk-new_archivers_lha/lha/work/lha-7c3cd95/config.log contains indications of -Wimplicit-function-declaration, check that features were not accidentially disabled:
Warning:   conftest.c:97:29: error: implicitly declaring library function 'exit' with type 'void (int) __attribute__((noreturn))' [-Werror,-Wimplicit-function-declaration]
Warning:   conftest.c:102:9: error: implicitly declaring library function 'exit' with type 'void (int) __attribute__((noreturn))' [-Werror,-Wimplicit-function-declaration]

These are the checks for whether strchr()/strrchr() is 8bit clean and whether the 2nd argument of gettimeofday() is effective so they're not related to the CFStringEncodingBytesToUnicode issue, yet they should be fixed.

[ryandesign]

No they are two different things. After running configure my config.h contains:

/* Define to 1 if you have the `iconv' function. */
/* #undef HAVE_ICONV */

/* Define to 1 if you have the <iconv.h> header file. */
#define HAVE_ICONV_H 1

I think it's specifically checking for the "iconv" function, which would only be available with a system copy of iconv or libiconv; in a third-party libiconv installation, the function is instead named "libiconv". These differences are supposed to be transparent to the program as long as it includes the headers properly; maybe that's not being done in the configure test here.

[mitchblank]

I posted an upstream fix for those conftest.c issues here -- jca02266/lha#18

[chrstphrchvz]

Should upstream still consider adding private function declarations, for those not using (lib)iconv?

[ryandesign]

macOS includes iconv as far back as Mac OS X 10.6, probably earlier, so I suspect the fallback code that uses private macOS functions will only be used on ancient OS versions. The code was added in June 2002. At that time Mac OS X 10.1 was the most recent version. Maybe it did not include iconv.

When the code was originally added in jca02266/lha@4ccf94a, it did include the prototypes of the private functions. The prototypes were removed in jca02266/lha@0b2a218 in July 2002 but the commit message doesn't really explain why. You could suggest that they re-add them.

[mitchblank]

The only reason I can think of to stick with private OS/X-specific APIs for converting filenames is if they help somehow with HFS's use of NFD-normalized unicode... although I don't know if those APIs do anything special in that regard (and it's thankfully disappearing from the world anyway since APFS stores non-normalized UTF-8 filenames like a normal UNIX fllesystem, albeit one that's case-insensitive by default)

If it's just doing basic UTF-8 <-> UTF-16 conversion you'd be better off just sticking with iconv I would say.