Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mpack: fix configure and build failures #27778

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

frankdean
Copy link
Contributor

Description

configure was failing as its tests appear to be pre C99 requiring implicit int.

Makefile.am failed to build due to a local variable being null.

Type(s)
  • bugfix
  • enhancement
  • security fix
Tested on

macOS 15.3.1 24D70 arm64
Xcode 16.2 16C5032a

Verification

Have you

Copy link
Contributor

@ryandesign ryandesign left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an open ticket for this problem. The commit message should conclude with the lines


Closes: https://trac.macports.org/ticket/71604

so that Trac will auto-close the ticket when the PR is merged.

@@ -2,7 +2,7 @@ PortSystem 1.0

name mpack
version 1.6
revision 1
revision 2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As explained in the guide, don't increase the revision just to fix a build failure since nothing has changed for anyone (i.e. those with older less-picky compilers) who was already able to install the port (unless using autoreconf causes more changes for this software than I'm used to).

Suggested change
revision 2
revision 1

patch-TMPDIR.diff

configure.args --mandir=${prefix}/share/man

use_autoreconf yes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a good idea to add a comment to explain why we are autoreconfing. That way it's easier to determine in future updates of the port whether it can be removed.

Suggested change
use_autoreconf yes
# Regenerate configure to fix implicit int errors.
# Also we are patching Makefile.am.
use_autoreconf yes

Comment on lines 23 to 24

configure.args --mandir=${prefix}/share/man
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

${prefix}/share/man is the default value for mandir in recent versions of autotools. Now that you're autoreconfing, there's no need to specify this.

Suggested change
configure.args --mandir=${prefix}/share/man

mpack_SOURCES=unixpk.c encode.c codes.c magic.c unixos.c xmalloc.c \
md5c.c
-mpack_LDADD=@LIBOBJS@ @LIB_SOCKET@
+mpack_LDADD=@LIBOBJS@
Copy link
Contributor

@ryandesign ryandesign Mar 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the error message you got before making this change? Are we sure this is the best fix?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the LIB_SOCKET wasn't being set at all, even to a null string, the value was being passed on the CLI causing a build failure...

clang: error: no such file or directory: '@LIB_SOCKET@'

The variable should have been set by a local m4 macro. The macro wasn't actually being called, but when it is called, it causes more issues as it sets incorrect values for LIBS on systems which already have the functions implemented in the standard library. I've implemented a more theoretically complete fix although the end result is the same but tidier.

Configure was failing as its tests appear to be pre C99 requiring implicit
int.

Makefile.am failed to build due to a local variable being null.  Fixed by
patching the autotools m4 macro that sets the variable.

Closes: https://trac.macports.org/ticket/71604
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants