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

add official bzip2 and curl pc files names #25

Closed
wants to merge 2 commits into from

Conversation

talregev
Copy link
Contributor

@talregev talregev commented Jan 30, 2025

@talregev talregev changed the title add official pc file name of bzip2 add official bzip2 and curl pc files names Jan 30, 2025
@mmuetzel
Copy link
Member

Thank you for your contribution.
I haven't tested so far. But these changes are looking good to me. 👍

I'd like to push the changes you are proposing to the (upstream) Mercurial repository. For that, I'd need your name and a valid email address. Could you please share that information?
If you prefer to stay anonymous, I could also push those changes with me in the author field. Please, let me know how you would like to proceed.

@talregev
Copy link
Contributor Author

talregev commented Jan 30, 2025

May I propose a different PR that everytime user will make a PR here, it will trigger the ci?
I can do these changes myself.

@mmuetzel
Copy link
Member

I will delete these information after your commit to the upstream, because I don't want bots will finds my email.

I don't know yet when I'll be able to push your changes. I'll try to do it some time this week.
But I have your information locally now. So, feel free to delete it from here whenever you like.

@talregev
Copy link
Contributor Author

I will delete these information after your commit to the upstream, because I don't want bots will finds my email.

I don't know yet when I'll be able to push your changes. I'll try to do it some time this week. But I have your information locally now. So, feel free to delete it from here whenever you like.

Thank you!

@mmuetzel
Copy link
Member

I slightly adapted your changes to Octave's coding style, added GNU-style commit messages and pushed the patches with you as the author upstream. They should be appearing shortly in this repository.

Thanks again for your contribution.

@mmuetzel mmuetzel closed this Jan 30, 2025
siko1056 pushed a commit that referenced this pull request Jan 30, 2025
* configure.ac: The pkg-config file for the bzip2 library is named "bzip2.pc".

See: #25
siko1056 pushed a commit that referenced this pull request Jan 30, 2025
* configure.ac: The pkg-config file for the curl library is named "libcurl.pc".

See: #25
@dasergatskov
Copy link
Contributor

@talregev talregev deleted the TalR/bzip2_pc_name branch February 1, 2025 03:52
@talregev
Copy link
Contributor Author

talregev commented Feb 1, 2025

I don't have macos to test octave on it.
On the ci with my commit, the macos looks green:
https://github.com/gnu-octave/octave/actions/runs/13058330610/job/36436677481
https://github.com/gnu-octave/octave/actions/runs/13058330610/job/36436677875
https://github.com/gnu-octave/octave/actions/runs/13058330610/job/36436678178
Can you add on the ci a test that detect that break?

In the discussion you found working solution:

I installed curl from homebrew, specified PKG_CONFIG_PATH=/opt/homebrew/opt/curl/lib/pkgconfig
and now everything works again.

Dmitri.

https://savannah.gnu.org/bugs/?66745#comment5

I am not sure how to tell configure to detect correctly the path for pkg config path.
If not solution is found, you can revert the change for libcurl, until solution is found also for macos.

@talregev
Copy link
Contributor Author

talregev commented Feb 1, 2025

Taken from discussion.

<   CURL LDFLAGS:                  -L/usr/lib
---
>   CURL LDFLAGS:
66262c66266
< CURL_LDFLAGS='-L/usr/lib'
---
> CURL_LDFLAGS=''

This curl you compile from source code?

@talregev
Copy link
Contributor Author

talregev commented Feb 1, 2025

From the ci:

2025-01-30T18:55:01.5069870Z   CURL CPPFLAGS:                 
2025-01-30T18:55:01.5070040Z   CURL LDFLAGS:                  
2025-01-30T18:55:01.5070210Z   CURL libraries:                -lcurl

In the ci, is the curl detected from .pc file?

@mmuetzel
Copy link
Member

mmuetzel commented Feb 1, 2025

In the ci, is the curl detected from .pc file?

If I understand correctly, it is:

2025-01-31T22:10:49.2059190Z configure:97714: $PKG_CONFIG --exists --print-errors "libcurl"
2025-01-31T22:10:49.2059230Z configure:97717: $? = 0

I don't know which curl is installed on that runner. There are apparently multiple options for macOS...

@talregev
Copy link
Contributor Author

talregev commented Feb 1, 2025

I will add prints

@dasergatskov
Copy link
Contributor

dasergatskov commented Feb 1, 2025

I don't know which curl is installed on that runner. There are apparently multiple options for macOS...

It must be the system curl. The one from Homebre is installed "keg-only", i.e. one needs to explicitly specify bin, lib, includes, or PKG_CONFIG_PATH to use it. So all CI builds are probably have the same problem. But since we do not have a BIST to test readline we do not know that.

@mmuetzel
Copy link
Member

mmuetzel commented Feb 1, 2025

So all CI builds are probably have the same problem.

I'm not sure they do. If I understood your analysis on Savannah correctly, the issue arises because pkg-config --libs-only-L libcurl returns -L/usr/lib for you (which it shouldn't).
That doesn't seem to be the case for the CI runners on macOS. According to the configure summary, $CURL_LDFLAGS is empty for them.

At the moment, I'm inclined to think that it is an issue with your pkg-config (or its configuration). If I understand correctly, it shouldn't return flags to system libraries unless they are explicitly requested (e.g., with pkg-config --libs-only-L --keep-system-libs libcurl).
If it returns system libraries (i.e., -L/usr/lib) for you without --keep-system-libs, pkg-config might not be configured correctly...

@mmuetzel
Copy link
Member

mmuetzel commented Feb 1, 2025

@talrgev kindly confirmed that the libcurl.pc that is used in the CI on macOS is the same that is also used on @dasergatskov's system:

  configure:97741: $PKG_CONFIG --exists --print-errors "libcurl"
  configure:97744: $? = 0
  configure:97755: found libcurl pc file in location "/usr/local/Homebrew/Library/Homebrew/os/mac/pkgconfig/13" version: 7.87.0

Yet, $CURL_LDFLAGS is empty in CI:

    CURL CPPFLAGS:                 
    CURL LDFLAGS:                  
    CURL libraries:                -lcurl

So, the issue that @dasergatskov is seeing is likely something local to his system.

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

Successfully merging this pull request may close these issues.

3 participants