Skip to content
New issue

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

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

Already on GitHub? # to your account

Use pkg-config to get x11 and xt flags #3

Merged
merged 13 commits into from
Jun 10, 2022

Conversation

zmughal
Copy link
Contributor

@zmughal zmughal commented Oct 24, 2021

This PR is on top of the commits of #2.

As discussed in #2, some
package managers (e.g., Homebrew) do not place x11 and xt files in the standard
locations. This uses pkg-config to get that information first.

configure.ac Show resolved Hide resolved
@zmughal
Copy link
Contributor Author

zmughal commented Oct 30, 2021

I have rebased on top of the main branch.

I also added a commit to test using the --without-x option.

@zmughal zmughal requested a review from djerius November 6, 2021 18:47
Copy link

@oodler577 oodler577 left a comment

Choose a reason for hiding this comment

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

defering judgement; looked at this mainly for my own edification and if I could see something obvious :)

.github/workflows/ci.yml Show resolved Hide resolved
@zmughal
Copy link
Contributor Author

zmughal commented Dec 12, 2021

@djerius, have a moment to look over this?

configure.ac Outdated Show resolved Hide resolved
@zmughal zmughal force-pushed the pkg-config-x11-xt branch 2 times, most recently from 7b858e2 to c78c98c Compare January 5, 2022 08:02
@zmughal
Copy link
Contributor Author

zmughal commented Jan 5, 2022

@djerius, now that change should work. More details in the commit:

Set macOS LIBRARY_PATH for Homebrew

@zmughal zmughal requested a review from djerius January 5, 2022 08:05
zmughal and others added 2 commits January 5, 2022 03:10
Co-authored-by: djerius <djerius@cfa.harvard.edu>
Per the macOS ld(1) man page <https://www.unix.com/man-page/OSX/1/ld/>,

> The default library search path is /usr/lib then /usr/local/lib.

When `HOMEBREW_PREFIX=/usr/local`, this is where the `.dylib` files for
X11 are symlinked by Homebrew.

When `AC_PATH_XTRA` is called, it uses `/usr/bin/gcc` (Apple clang) and
since the libraries are found in `/usr/local/lib`, it does not need to
specify any `-L` paths.

However when `libtool` is used to call Homebrew `gfortran`, it sets
the flag `-syslibroot` which per the man page for ld(1):

> The -syslibroot option will prepend a prefix to all search paths.

and this means that it no longer looks under `/usr/local/lib` by
default. To fix this, add the default path back by setting the
`LIBRARY_PATH` environment variable.
@zmughal zmughal force-pushed the pkg-config-x11-xt branch 2 times, most recently from 98f1723 to bb4bc01 Compare January 5, 2022 08:14
@zmughal zmughal force-pushed the pkg-config-x11-xt branch 4 times, most recently from 3cbd03b to 7515756 Compare January 13, 2022 02:56
This is to test that if libraries are not symlinked into the Homebrew
prefix (here by using `brew unlink`), then `autotool` will use
`pkg-config` to find the libraries.
@zmughal
Copy link
Contributor Author

zmughal commented Jan 15, 2022

@djerius, the latest commits should now test the case where AC_PATH_XTRA fails and must fallback on pkg-config. I also added the extra parameter to PKG_CHECK_MODULES so that it does not error out of the script if pkg-config can not find x11.

@djerius djerius merged commit 70ce353 into djerius:main Jun 10, 2022
@djerius
Copy link
Owner

djerius commented Jun 10, 2022

Sorry about the delay!

@zmughal
Copy link
Contributor Author

zmughal commented Jun 10, 2022

Not a problem!

I was wondering if the next thing to tackle is getting #1 incorporated (hopefully it can be done and the license can be worked out).

@zmughal zmughal deleted the pkg-config-x11-xt branch June 10, 2022 21:13
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants