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

build: Fix Linux builds #199

Merged
merged 2 commits into from
Jun 12, 2023
Merged

build: Fix Linux builds #199

merged 2 commits into from
Jun 12, 2023

Conversation

jtojnar
Copy link
Contributor

@jtojnar jtojnar commented May 18, 2023

No description provided.

@Lymphatus
Copy link
Owner

I feel like that these changes may break the build on Mac. I'll run a few tests manually because there's no macOS in the Github workflow yet.

@Lymphatus Lymphatus closed this May 25, 2023
@Lymphatus Lymphatus reopened this May 25, 2023
@jtojnar
Copy link
Contributor Author

jtojnar commented May 25, 2023

I do not think this affects MacOS. It just stops building libcaesium as a DLL file (which I assume is useless on MacOS just like on Linux). And removes cargo, as it was only used for the aforementioned libcaesium build.

@Lymphatus
Copy link
Owner

Right now in the main branch the checks for macOS fail, but with this PR it fails almost instantly in the process.
I think a better way of handling the libcaesium dependency is to check whether the library already exists in the system and, if not, invoke the build. On macOS, like Windows, there's no way to install libcaesium except manually (no brew for example) so most of the time it's needed. But in a case like your, where the library is installed by other means, the build is indeed useless.
I'll work on it.

PS: the script is a bit misleading because of BUILD_BYPRODUCTS, but the ExternalProject actually builds a .so file for UNIX systems, not a .dll.

`ExternalProject_Add` does not work in build sandboxes without network access Linux distros like NixOS use.
Let’s skip it and use `libcaesium` from package manager when available, since that is the preferred way on Linux.
Also rename it so that it does not contain spaces.
@jtojnar
Copy link
Contributor Author

jtojnar commented May 25, 2023

I think a better way of handling the libcaesium dependency is to check whether the library already exists in the system and, if not, invoke the build

Did that.

On macOS, like Windows, there's no way to install libcaesium except manually (no brew for example) so most of the time it's needed.

Hopefully, this will allow it to be packaged in Homebrew eventually.

@Lymphatus Lymphatus merged commit afaedac into Lymphatus:main Jun 12, 2023
2 of 3 checks passed
@jtojnar jtojnar deleted the linux-build branch June 12, 2023 18:19
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