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 option for static linking via --enable-static #16

Merged
merged 1 commit into from
Sep 9, 2021

Conversation

mzpqnxow
Copy link
Contributor

@mzpqnxow mzpqnxow commented Sep 5, 2021

Hello, I'm not sure if you (or anyone) is an authoritative maintainer of CNTLM at this point- but your fork was the most up to date one I could find. If you're interested, I added an --enable-static flag to configure file as well as the Makefile

I'm using this branch with a musl-libc toolchain to statically link cntlm with libpacparser support. It's a bigger file of course but doesn't require any dependencies (or require any specific versions of any dependencies) which is useful in my environment

Thanks

(You'll see I also removed an orphaned, commented out debug symbols flag line, the debug symbols are set higher up in the file in a proper conditional block so it seems it's just old clutter)

@jschwartzenberg
Copy link
Collaborator

I guess that debug statement was my mistake. And yeah this support seems quite useful. Having to add the lib and a script that sets LD_LIBRARY_PATH can be a bit clumsy and this is much more practical. I hope to test this at least and merge when it's ok. Maybe @versat will check too. Thanks!

@mzpqnxow
Copy link
Contributor Author

mzpqnxow commented Sep 6, 2021

Great, thanks for being the steward of this project- and being open to contributions, albeit small ones :)

I should explicitly state that I did not do any formal/rigorous testing of this, aside from typical use in my environment- which is not as heterogeneous as I like to pretend (only different distributions of Linux on x86_64 and ppc64le)

Of particular note, I didn't test this at all on MacOS/gcc or Windows/mingw as I don't have a Mac or a Windows system on-hand

For such a small change like this, the testing is probably the largest burden so thanks for considering taking that on. I'll keep this open in case there's anything I can do to help. I could easily see this not working as expected on mingw or MacOS gcc...

Thanks again

@jschwartzenberg
Copy link
Collaborator

I should explicitly state that I did not do any formal/rigorous testing of this, aside from typical use in my environment- which is not as heterogeneous as I like to pretend (only different distributions of Linux on x86_64 and ppc64le)

I suspect you're the first to test this on ppc64le :)

Of particular note, I didn't test this at all on MacOS/gcc or Windows/mingw as I don't have a Mac or a Windows system on-hand

As far as I know, there's no MinGW port for Windows. I am only aware of compilation for Windows on Cygwin. A non-Cygwin port would be seem useful though. @versat did you ever look into this?

For such a small change like this, the testing is probably the largest burden so thanks for considering taking that on. I'll keep this open in case there's anything I can do to help. I could easily see this not working as expected on mingw or MacOS gcc...

The only variation I was thinking of was including also Kerberos instead of (only) pacparser :) As long as it works properly on at least one platform I think it will be useful to merge. I am afraid the other features (at least Kerberos & PAC support) might not work on those either, I mostly tested on RHEL7. I looked at building pacparser itself on Cygwin, but that didn't work "just like that".

Great, thanks for being the steward of this project- and being open to contributions, albeit small ones :)

The main person here is @versat! So far I've mainly been contributing other people's work that was scattered around the web :)

@versat
Copy link
Owner

versat commented Sep 7, 2021

Hello, I'm not sure if you (or anyone) is an authoritative maintainer of CNTLM at this point- but your fork was the most up to date one I could find.

Hi, this is no official repository, I am not an authoritative maintainer or so, this is just another fork :)

Thanks for the PR. It looks good to me. Since it is just an option I do not see any problem merging this. We could extend the automated tests to build it with this option too.

@versat
Copy link
Owner

versat commented Sep 7, 2021

As far as I know, there's no MinGW port for Windows. I am only aware of compilation for Windows on Cygwin. A non-Cygwin port would be seem useful though. @versat did you ever look into this?

No, I have only built it via Cygwin.

@versat versat merged commit d546bfe into versat:master Sep 9, 2021
@mzpqnxow
Copy link
Contributor Author

mzpqnxow commented Sep 11, 2021

I suspect you're the first to test this on ppc64le :)

Hah, yes. I suspect you're correct on this point

Thank you for accepting and merging!

@mzpqnxow
Copy link
Contributor Author

FYI- I submitted a PR to the pacparser project so it can produce a static library as well, since I needed PAC support in my statically linked CNTLM

The PR hasn't been merged but if anyone needs to do this, the PR (and reference to my fork/branch is here

Copy link

sonarcloud bot commented Feb 12, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

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.

None yet

3 participants