Skip to content

Configure: fixed --with-libatomic=DIR with recent libatomic_ops. #460

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

Merged
merged 1 commit into from
Jan 30, 2025

Conversation

pluknet
Copy link
Contributor

@pluknet pluknet commented Jan 17, 2025

The build location of the resulting libatomic_ops.a was changed in v7.4.0 after converting libatomic_ops to use libtool. The fix is to use headers and library from the install prefix, similar how we do this with OpenSSL. This approach allows building with both old and new library versions.

Initially reported here:
https://mailman.nginx.org/pipermail/nginx/2018-April/056054.html

Copy link
Member

@bavshin-f5 bavshin-f5 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 a LINK_DEPS entry without a corresponding target. CORE_DEPS already ensures the build order, so we can drop the library dependency.

With some make implementations (NetBSD 9.x make with compatibility mode off specifically), this results in

make: make: don't know how to make ../lib/libatomic_ops-7.2/build/lib/libatomic_ops.a. Stop

@pluknet
Copy link
Contributor Author

pluknet commented Jan 29, 2025

Sure, thanks.

Caught by FreeBSD's make (which essentially NetBSD's make) in parallel build as well.

Copy link
Member

@bavshin-f5 bavshin-f5 left a comment

Choose a reason for hiding this comment

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

Looks close to what we already do for --with-openssl= and works on all the systems I tried.
LGTM!

@pluknet
Copy link
Contributor Author

pluknet commented Jan 29, 2025

Changes:

  • restored CORE_DEPS to LINK_DEPS (in anticipation of win32 support)
  • fixed indentation
  • restored a separate $NGX_LIBATOMIC/Makefile make target, mostly to reduce unrelated changes

@pluknet
Copy link
Contributor Author

pluknet commented Jan 29, 2025

Looks close to what we already do for --with-openssl= and works on all the systems I tried. LGTM!

I've once again changed Makefile generation, back to LINK_DEPS.
Aside from minimizing changes, it will make win32 changes similar, since there's no install target in Makefile.msft.

@pluknet
Copy link
Contributor Author

pluknet commented Jan 29, 2025

@bavshin-f5 please review now that I've added win32 support.

@bavshin-f5
Copy link
Member

First patch looks good.
win32 support patch is either unnecessary or incomplete, because src/os/win32/ngx_atomic.h does not support using libatomic.

The build location of the resulting libatomic_ops.a was changed in v7.4.0
after converting libatomic_ops to use libtool.  The fix is to use library
from the install path, this allows building with both old and new versions.

Initially reported here:
https://mailman.nginx.org/pipermail/nginx/2018-April/056054.html
@pluknet
Copy link
Contributor Author

pluknet commented Jan 30, 2025

First patch looks good. win32 support patch is either unnecessary or incomplete, because src/os/win32/ngx_atomic.h does not support using libatomic.

Let's put win32 support aside for now, unless there will be a demand.

@pluknet pluknet merged commit e715202 into nginx:master Jan 30, 2025
1 check passed
@pluknet pluknet deleted the libatomic branch January 30, 2025 13:17
@Maryna-f5 Maryna-f5 added this to the nginx-1.27.4 milestone Jan 30, 2025
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