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

xft2-dev patch to complie under macOS 15 / CLT 16.1 #1169

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

sth0
Copy link
Contributor

@sth0 sth0 commented Oct 31, 2024

Needed cast of NULL to (unsigned long) in xftdpy.c to comply with stricter compiler requirements.

Needed cast of NULL to (unsigned long) in xftdpy.c
@sth0 sth0 added the bug label Oct 31, 2024
Copy link
Contributor

@dhomeier dhomeier left a comment

Choose a reason for hiding this comment

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

Does build on 14.7/Xcode 16.1 as well; however I was also able to build the latest version 2.3.8 without needing to patch any additional files; the bug in https://bugs.freedesktop.org/show_bug.cgi?id=47178 seems to have long been fixed upstream.

@sth0
Copy link
Contributor Author

sth0 commented Oct 31, 2024

I will try the newer upstream version, I just didn't want to get into recreating a complex patch file.

@sth0
Copy link
Contributor Author

sth0 commented Nov 1, 2024

Updated to use upstream 2.3.8 version. Only tested on Intel 15.X system

Copy link
Contributor

@dhomeier dhomeier left a comment

Choose a reason for hiding this comment

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

looks good apart from the Shlibs-version; built on 14.7/Xcode 16.1-arm64.
I originally tried with the old AM_CPPFLAGS patch (the freetype one in xftglyphs.c is already merged upstream), but that does not seem to be needed anymore either – compiled with -I../include/X11/Xft in front in any case.

10.9-libcxx/stable/main/finkinfo/x11/xft2-dev.info Outdated Show resolved Hide resolved
@sth0
Copy link
Contributor Author

sth0 commented Nov 2, 2024

After reading the shlibs policy, I am not sure why this package would be compatibility version 6.0.0 vs 5.0.0 and not 2.0.0. What sets that version number? Is it just an incremented value as the shlib version is changed? Why would it be a major change instead of a minor change to reflect the actual code revision version?

@dhomeier
Copy link
Contributor

dhomeier commented Nov 5, 2024

The compatibility version is whatever otool -L says it is, which is set upstream in the code, so not really within the powers of our packaging policy. When and how upstream developers decide to increment it is usually beyond my understanding, so I generally build with fink --validate.

@nieder
Copy link
Member

nieder commented Nov 10, 2024

Is the spacing bug mentioned in the unpatched .info (now at https://gitlab.freedesktop.org/xorg/lib/libxft/-/issues/1 ) fixed? @dmacks

ETA: noticed @dhomeier saying https://bugs.freedesktop.org/show_bug.cgi?id=47178 was fixed. That bug was only closed because upstream moved from bugzilla to gitlab. The moved current bug URL is at my link above.

@nieder
Copy link
Member

nieder commented Nov 10, 2024

Doesn't matter since I think the NULL fix is fine, but upstream changed it to 0, rather than typing it.
https://gitlab.freedesktop.org/xorg/lib/libxft/-/commit/9c23173cf1ff861bdb8538e3aa21ec509b0d87d8

I cherry picked the first commit (NULL fix) and applied it to master.

@sth0
Copy link
Contributor Author

sth0 commented Nov 10, 2024

I thought I had reverted everything except the NULL fix. But anyway, thanks, and I think which value is cast to what is irrelevant. I personally prefer keeping the NULL for readability of the code, but YMMV.

@nieder
Copy link
Member

nieder commented Nov 10, 2024

Yeah, I don't care if it's a typed NULL or 0. Anyway, I have no idea why this PR shows conflicts since I specifically cherry picked the first commit in the PR.

@sth0
Copy link
Contributor Author

sth0 commented Nov 10, 2024

I see the difference, the update to the newer upstream as suggested by dhomeier was rejected. 2.3.8 vs 2.3.0

@sth0
Copy link
Contributor Author

sth0 commented Nov 11, 2024

Been traveling, seems like I didn't push the reversion to the previous Shlibs def. Now done.

@nieder
Copy link
Member

nieder commented Nov 12, 2024

From the build output during deb file validation at the end:

Validating .deb dir /opt/sw/build.build/root-xft2-shlibs-2.3.8-1...
Error: Shlibs field says compatibility version for /opt/sw/lib/xft2/lib/libXft.2.dylib is 5.0.0, but it is actually 6.0.0.

The values before 928321f were actually correct.

@dhomeier
Copy link
Contributor

The previous ones were those for 2.3.8, these are now the 2.2.0 ones again, but I thought the update to 2.3.8 was rejected since it is not clear if the spacing problem in https://gitlab.freedesktop.org/xorg/lib/libxft/-/issues/1 has really been fixed?
There is one newer commit https://gitlab.freedesktop.org/xorg/lib/libxft/-/commit/3ca7a7c that sounds like it might be dealing with it, but I haven't been able to confirm that – did not notice any fonts problems with my xterm, but it does not look like that is linking xft2.

@dhomeier
Copy link
Contributor

dhomeier commented Nov 12, 2024

I did build mrxvt now once against the 2.2.0 version from your initial commit, and against 2.3.8 without the patches, and cannot see any differences in font spacings or other issues between the two:
mrxvt-xft2-2 2 0

mrxvt-xft2-2 3 8

So updating might be OK? Also built on 10.14 with Xcode 11.3 without the patchfile.

sth0 added 2 commits November 14, 2024 21:29
# Conflicts:
#	10.9-libcxx/stable/main/finkinfo/x11/xft2-dev.info
#	10.9-libcxx/stable/main/finkinfo/x11/xft2-dev.patch
% otool -L /opt/sw/lib/xft2/lib/libXft.2.3.8.dylib
/opt/sw/lib/xft2/lib/libXft.2.3.8.dylib:
		/opt/sw/lib/xft2/lib/libXft.2.dylib (compatibility version 6.0.0, current version 6.8.0)
@sth0
Copy link
Contributor Author

sth0 commented Nov 15, 2024

Set compatibility version to 6.0.0 as tool declares it
Resolved merge with master, this should be working.

% otool -L /opt/sw/lib/xft2/lib/libXft.2.3.8.dylib
/opt/sw/lib/xft2/lib/libXft.2.3.8.dylib:
/opt/sw/lib/xft2/lib/libXft.2.dylib (compatibility version 6.0.0, current version 6.8.0)

@sth0 sth0 requested a review from dhomeier December 14, 2024 20:26
@sth0
Copy link
Contributor Author

sth0 commented Jan 3, 2025

Should be good to go?
image

Patchfile necessary for stringent compiler on Sequoia

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants