Skip to content
This repository has been archived by the owner on Jan 15, 2025. It is now read-only.

(PA-5925) rectified option in openssl1.1.1 #753

Merged

Conversation

amitkarsale
Copy link
Contributor

rectified the compiler option.

@amitkarsale amitkarsale requested review from a team as code owners November 1, 2023 10:39
@@ -45,7 +45,7 @@
pkg.environment 'CC', '/opt/pl-build-tools/bin/gcc'

cflags = '$${CFLAGS} -static-libgcc'
ldflags = "#{settings[:ldflags]} -WL, -R#{settings[:libdir]}"
ldflags = "#{settings[:ldflags]} -Wl, -R#{settings[:libdir]}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Were you able to build the runtime with this change? The reason I ask is I'm not used to seeing spaces between -Wl and the rest:

Suggested change
ldflags = "#{settings[:ldflags]} -Wl, -R#{settings[:libdir]}"
ldflags = "#{settings[:ldflags]} -Wl,-R#{settings[:libdir]}"

The idea is -Wl tells gcc to pass the arguments that directly follow to the linker. But if there's a space after the comma it may not work as expected?

Copy link
Contributor

@joshcooper joshcooper Nov 1, 2023

Choose a reason for hiding this comment

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

Yeah the space doesn't work:

Makefile:111: recipe for target 'openssl-1.1.1-build' failed
gcc: error: unrecognized command line option '-R'
gmake[2]: *** [apps/app_rand.o] Error 1
gmake[1]: *** [all] Error 2
gmake: *** [openssl-1.1.1-build] Error 2

So I force pushed to your branch. It looks like your vanagon generic builder jobs built agent-runtime-main which uses openssl 3 so it didn't catch the problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am new to using gcc command options. For other commands I used spaces between the options so took it casually. thanks for catching this Josh! B-)

Prepend our lib directory to the dynamic library search path so that the
openssl binary can find its dependencies.
@joshcooper joshcooper force-pushed the PA-5925-openssl1.1.1 branch from 749687f to 921ac2c Compare November 1, 2023 17:44
Copy link
Contributor

@joshcooper joshcooper left a comment

Choose a reason for hiding this comment

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

# dump -H -X32 /opt/puppetlabs/puppet/bin/openssl 

/opt/puppetlabs/puppet/bin/openssl:

                        ***Loader Section***
                      Loader Header Information
VERSION#         #SYMtableENT     #RELOCent        LENidSTR
0x00000001       0x0000067a       0x00001875       0x00000099       

#IMPfilID        OFFidSTR         LENstrTBL        OFFstrTBL
0x00000006       0x0001c10c       0x000078d8       0x0001c1a5       


                        ***Import File Strings***
INDEX  PATH                          BASE                MEMBER              
0      /opt/puppetlabs/puppet/lib:/usr/lib:/lib                                         
1                                    libssl.a            libssl.so.1.1       
2                                    libcrypto.a         libcrypto.so.1.1    
3                                    libpthreads.a       shr_xpg5.o          
4                                    libc.a              shr.o               
5                                    librtl.a            shr.o   

@joshcooper joshcooper merged commit 52cc4cb into puppetlabs-toy-chest:master Nov 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants