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

Fix linker ssl error fix (libcrypto-1_1-x64.dll not found) #9

Merged
merged 14 commits into from
Jun 4, 2024

Conversation

IsaacShoebottom
Copy link

Can't make an issue so I thought I'd drop this here, since upstream is inactive and you seem to be around more often.

For some reason Aseprite wants to link against libcrypto (libcrypto-1_1-x64.dll) at runtime if OpenSSL isn't present at build time. Possible a fix in tooling/toolchains that disrupted behavior. There has been some problems123 with OpenSSL in the past and just installing the full OpenSSL from chocolaty seemed to fix the build for me.

It may be there is an alternate fix by setting env variables to point to a preinstalled system OpenSSL, as the windows runner claims4 it has OpenSSL 1.1.1w installed, but cmake/ninja don't find it.

Code in PR is included merely as reference as to my fix, no need to merge if you think you can do better.

Footnotes

  1. https://github.com/actions/runner-images/issues/371

  2. https://github.com/actions/runner-images/issues/8344

  3. https://github.com/actions/runner-images/issues/9395

  4. https://github.com/actions/runner-images/blob/main/images/windows/Windows2022-Readme.md#tools

@IsaacShoebottom
Copy link
Author

It would also might make sense to change the default dependencies action for windows:

- name: (Windows) Install dependencies
  if: matrix.os == 'windows-latest'
  uses: seanmiddleditch/gha-setup-ninja@v3

action and merge it with installing OpenSSL and ninja through chocolatey like so:

- name: (Windows) Install dependencies
  if: matrix.os == 'windows-latest'
  shell: powershell
  run: choco install openssl ninja

as the above action installs ninja by running NodeJS 16, which is soon to be deprecated and when installing through chocolatey, the binary is newer

@IsaacShoebottom
Copy link
Author

After some more investigation, I have found that for some reason, CMake decides to link against OpenSSL if it's detected. This is not needed, at least from what I can tell, news still works, you don't need updates on portable builds and DRM isn't needed either. So you can just remove OpenSSL from the default image to prevent it from trying to link, and everything works, as far as I can tell

(Sidenote, installing OpenSSL only worked for me because I had version 3.3.0 installed on my system already, but it would not work in an default system, so don't use that patch)

@pedroterzero
Copy link
Owner

pedroterzero commented Jun 2, 2024

Hi, @IsaacShoebottom, thanks for the PR.

However, I just tried building without any changes and it still seems to work for me:

https://github.com/pedroterzero/aseprite_builder/actions/runs/9337348474/job/25699118258

Perhaps I'm misunderstanding. Do you mean it builds successfully but things don't work properly with the resulting binary?

@IsaacShoebottom
Copy link
Author

IsaacShoebottom commented Jun 2, 2024

Perhaps you have libcrypto-1_1.dll installed on your system path. You can see people on the original PR you submitted to the original aseprite builder repo mention an error about libcrypto-1_1.dll.

You can see what dlls your builds load at runtime with a tool like this. If you look at your build vs my pr you will probably see the difference. I'm not sure why it's even linking against libcrypto in the first place, but the fix for people who don't have it already is just to install OpenSSL 1.1.1w, if you don't prevent cmake from finding OpenSSL at build time, so you could just put a link in the readme to the correct OpenSSL version, which is this at time of writing. (The reason for light instead of full is that full includes build headers not needed for runtime linking)

@pedroterzero
Copy link
Owner

pedroterzero commented Jun 2, 2024

I'm not doing this on my local machine, I don't even have windows 😉

In the pipeline it seems to work as is. Are you not using the pipeline to build?

Or do you mean that it builds successfully but then doesn't work on windows? I'm slightly confused 😉

@IsaacShoebottom
Copy link
Author

It builds correctly but does not work on windows, yes. It's a link error at runtime.

@pedroterzero pedroterzero merged commit 7eff0f2 into pedroterzero:master Jun 4, 2024
@pedroterzero
Copy link
Owner

It builds correctly but does not work on windows, yes. It's a link error at runtime.

Ahh, I see then. Well, if this fixes it, great. I'll merge it. Thanks!

@IsaacShoebottom IsaacShoebottom deleted the link-ssl-error branch June 6, 2024 04:50
Ecorous added a commit to Ecorous/aseprite_builder that referenced this pull request Jul 10, 2024
Fix linker ssl error fix (libcrypto-1_1-x64.dll not found) (pedroterzero#9)
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

2 participants