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

curl-config requires the bc command to work #26281

Open
embray opened this issue Sep 14, 2018 · 11 comments
Open

curl-config requires the bc command to work #26281

embray opened this issue Sep 14, 2018 · 11 comments

Comments

@embray
Copy link
Contributor

embray commented Sep 14, 2018

As we found out in #24919 (see #24919 comment:63) the curl-config program that comes with curl, which we use to check the curl version, requires the standard bc command, which apparently on some systems is missing?

In a way this just means that the packaging of curl is broken since it should list bc as a runtime dependency. But we can work around that by checking for it ourselves.

This issue should be independent of #24919 since we already use curl-config to check for curl...

Upstream: Fixed upstream, in a later stable release.

CC: @infinity0

Component: packages: standard

Issue created by migration from https://trac.sagemath.org/ticket/26281

@dimpase
Copy link
Member

dimpase commented Oct 15, 2018

Upstream: Not yet reported upstream; Will do shortly.

@dimpase
Copy link
Member

dimpase commented Oct 15, 2018

comment:1

This looks like a Debian bug, as I see this on Debian, and they package (lib)curl.
By right, curl-config they ship must be fully functional, but it misses bc as a dependency.

I cc one of Debian people packaging Sagemath here.

@dimpase
Copy link
Member

dimpase commented Oct 28, 2018

comment:2

discussed with upstream on curl/curl#3143
and upstream fixed it in curl/curl#3174

@dimpase
Copy link
Member

dimpase commented Oct 28, 2018

Changed upstream from Not yet reported upstream; Will do shortly. to Fixed upstream, in a later stable release.

@embray
Copy link
Contributor Author

embray commented Oct 28, 2018

comment:3

Thanks.

@embray
Copy link
Contributor Author

embray commented Oct 28, 2018

comment:4

I'm not sure what we should do in sage, if anything. I suppose it might still be worth adding an explicit check for the bc program when installing curl :(

@dimpase
Copy link
Member

dimpase commented Oct 28, 2018

comment:5

Replying to @embray:

I'm not sure what we should do in sage, if anything. I suppose it might still be worth adding an explicit check for the bc program when installing curl :(

we could just make bc required in the top configure.ac, calling AC_CHECK_PROGRAM or whatever the right macro is.

@slel
Copy link
Member

slel commented Sep 16, 2020

comment:6

The upstream fix was merged in curl 7.62.0.

Rules for deciding whether to build curl are
described in build/pkgs/curl/spkg-configure.m4.

Currently the approach is based on R's needs:

  • build curl unless curl >= 7.22 is found.

Which of the following should we change it to?

  • build curl unless curl >= 7.62 is found
  • build curl unless either curl >= 7.62 is found or curl >= 7.22 and bc are found

@dimpase
Copy link
Member

dimpase commented Sep 16, 2020

comment:7

Currently we do not build curl if curl >= 7.22 (and, implcitly, bc - if the curl in question does need bc in its curl-config).

Adding an explicit check for bc (after our test for curl failed) would only improve error messages, but that's a rare event that a system has no bc installed.

Other potential improvements here would be to get rid of somewhat dodgy LIBCURL_CHECK_CONFIG and just use PKG_CHECK_MODULES (which doesn't need bc), or try PKG_CHECK_MODULES, and otherwise fall on LIBCURL_CHECK_CONFIG.

But all this is very minor.

@slel
Copy link
Member

slel commented Sep 16, 2020

comment:8

So maybe:

  • check for curl >= 7.62: if found, good
  • if not found, check for curl >= 7.22 and bc: if found, good
  • if not found: build curl

@dimpase
Copy link
Member

dimpase commented Sep 16, 2020

comment:9

Replying to @slel:

So maybe:

  • check for curl >= 7.62: if found, good
  • if not found, check for curl >= 7.22 and bc: if found, good
  • if not found: build curl

please see my edits

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

No branches or pull requests

3 participants