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 support for compiles on HPE NonStop #230

Closed
wants to merge 1 commit into from

Conversation

rsbeckerca
Copy link

Fixes: #229

@rsbeckerca
Copy link
Author

I still need help convincing configure to build a DLL. The arguments are not the same as for gcc and configure gets confused about it. -Wshared is the trigger to build .so files.

@eli-schwartz
Copy link
Collaborator

Looking at the relevant libtool logic it appears that it matches irix5* | irix6* | nonstopux* and defines $archive_cmds to use -shared. Rough summary:

      if test yes = "$GCC"; then
        archive_cmds='$CC -shared $pic_flag $libobjs $deplibs $compiler_flags $wl-soname $wl$soname `test -n "$verstring" && func_echo_all "$wl-set_version $wl$verstring"` $wl-update_registry $wl$output_objdir/so_locations -o $lib'
      else
        archive_cmds='$CC -shared $libobjs $deplibs $compiler_flags -soname $soname `test -n "$verstring" && func_echo_all "-set_version $verstring"` -update_registry $output_objdir/so_locations -o $lib'

@rsbeckerca
Copy link
Author

Looking at the relevant libtool logic it appears that it matches irix5* | irix6* | nonstopux* and defines $archive_cmds to use -shared. Rough summary:

      if test yes = "$GCC"; then
        archive_cmds='$CC -shared $pic_flag $libobjs $deplibs $compiler_flags $wl-soname $wl$soname `test -n "$verstring" && func_echo_all "$wl-set_version $wl$verstring"` $wl-update_registry $wl$output_objdir/so_locations -o $lib'
      else
        archive_cmds='$CC -shared $libobjs $deplibs $compiler_flags -soname $soname `test -n "$verstring" && func_echo_all "-set_version $verstring"` -update_registry $output_objdir/so_locations -o $lib'

nonstopux* is not the same. It is, in fact, deprecated - old product naming from when Compaq owned Tandem and made everything "NonStop". This is a separate set of platforms - nsx-tandem-nsk and nse-tandem-nsk are currently value entries from config.guess. -Wshared is what is needed.

@eli-schwartz
Copy link
Collaborator

:(

Those host identifiers aren't shown anywhere in libtool.m4 (which is used to generate the libtool script by configure) that I can tell.

My conclusion is that while config.guess knows the platform exists, libtool in particular doesn't actually do anything whatsoever with that information and does not know how to build libraries for nonstop. (This is ironic since the one selling point of libtool is supposed to be that it works on any platform ever.)

@rsbeckerca
Copy link
Author

Explains why libtool is a noop on the platform

@rsbeckerca
Copy link
Author

This PR also includes changes to some tests to honor HAVE_ALLOCA_H. The changes build and test correctly on the NonStop (TANDEM) x86 platform.

@@ -131,7 +131,11 @@ int main(int argc, const char * const *argv)

if (valgrind && *valgrind) {
size_t cmdsize = strlen(valgrind) + strlen(argv[0]) + 32;
#ifndef HAVE_ALLOCA_H
Copy link
Owner

Choose a reason for hiding this comment

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

I think we can just drop using alloca() and always use malloc.
If you don't mind, I'll prepare a patch for it and put you in as Reported-by:.

Copy link
Author

Choose a reason for hiding this comment

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

Works for me.

@rockdaboot
Copy link
Owner

@rsbeckerca Thanks for your contribution !
If we can get in #228, there should be no need to include network header files.
I'll come back to here once #228 has been closed or merged.

@rsbeckerca
Copy link
Author

rsbeckerca commented Feb 4, 2024

@rsbeckerca Thanks for your contribution ! If we can get in #228, there should be no need to include network header files. I'll come back to here once #228 has been closed or merged.

Let me know when you are ready. I can reroll the PR if you want. I think netdb.h is also required but not for #228.

@rockdaboot
Copy link
Owner

@rsbeckerca Removal of alloca: #231
Please drop that alloca commit from your PR (feel free to force-push).

@rsbeckerca
Copy link
Author

@rsbeckerca Removal of alloca: #231 Please drop that alloca commit from your PR (feel free to force-push).

Should be done. I repushed the original commit dropping e85cddd.

@rockdaboot
Copy link
Owner

@rsbeckerca Can you try to build and run tests from latest master without any changes?

@rsbeckerca
Copy link
Author

@rsbeckerca Can you try to build and run tests from latest master without any changes?

No, there are a few patches I have to apply to make this work. The summary of what I have to do from the tarball is as follows:

	        	sed "/<ws2tcpip.h>/a #elif defined __TANDEM" -i src/psl.c
	        	sed "/defined __TANDEM/a # include <unistd.h>" -i src/psl.c
	        	sed "/defined __TANDEM/a # include <netinet/in.h>" -i src/psl.c
	        	sed "/defined __TANDEM/a # include <netinet/in6.h>" -i src/psl.c
	        	sed "/defined __TANDEM/a # include <netdb.h>" -i src/psl.c
	        	sed "/defined __TANDEM/a # include <sys/socket.h>" -i src/psl.c

	        	sed "s/alloca(/(char *)malloc(/g" -i tests/test-is-cookie-domain-acceptable.c
	        	sed "s/alloca(/(char *)malloc(/g" -i tests/test-is-public-all.c
	        	sed "s/alloca(/(char *)malloc(/g" -i tests/test-is-public-builtin.c
	        	sed "s/alloca(/(char *)malloc(/g" -i tests/test-is-public.c
	        	sed "s/alloca(/(char *)malloc(/g" -i tests/test-registrable-domain.c

@rockdaboot
Copy link
Owner

rockdaboot commented Mar 24, 2024

@rsbeckerca Can you try to build and run tests from latest master without any changes?

No, there are a few patches I have to apply to make this work. The summary of what I have to do from the tarball is as follows:

	        	sed "/<ws2tcpip.h>/a #elif defined __TANDEM" -i src/psl.c
	        	sed "/defined __TANDEM/a # include <unistd.h>" -i src/psl.c
	        	sed "/defined __TANDEM/a # include <netinet/in.h>" -i src/psl.c
	        	sed "/defined __TANDEM/a # include <netinet/in6.h>" -i src/psl.c
	        	sed "/defined __TANDEM/a # include <netdb.h>" -i src/psl.c
	        	sed "/defined __TANDEM/a # include <sys/socket.h>" -i src/psl.c

	        	sed "s/alloca(/(char *)malloc(/g" -i tests/test-is-cookie-domain-acceptable.c
	        	sed "s/alloca(/(char *)malloc(/g" -i tests/test-is-public-all.c
	        	sed "s/alloca(/(char *)malloc(/g" -i tests/test-is-public-builtin.c
	        	sed "s/alloca(/(char *)malloc(/g" -i tests/test-is-public.c
	        	sed "s/alloca(/(char *)malloc(/g" -i tests/test-registrable-domain.c

Are you sure you are using latest git master? Because all usages of alloca() have been replaced/removed, as well as all the network related include directives.

Latest commit is

commit 15e7f407cbadf950410e45d97e76773b40377ac6 (HEAD -> master, origin/master, origin/HEAD)
Author: Tim Rühsen <[email protected]>
Date:   Sat Mar 23 18:16:17 2024 +0100

Just saw you are building from tarball... here the tarball with the latest changes:
libpsl-0.21.5-15e7f407.tar.gz

@rsbeckerca
Copy link
Author

I can build libpsl.a from the above tarball. Thanks. configure.ac needs a little tweeking to get a .so file, but I don't have a delivery requirement for that, so it is fine as is for now.

@rockdaboot
Copy link
Owner

I can build libpsl.a from the above tarball. Thanks. configure.ac needs a little tweeking to get a .so file, but I don't have a delivery requirement for that, so it is fine as is for now.

Thanks for testing. Then I close this issue and #229.

@rockdaboot rockdaboot closed this Mar 24, 2024
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.

Request broader platform support for libpsl
3 participants