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 VCPKG on Windows #493

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

aljen
Copy link

@aljen aljen commented Dec 16, 2023

  • I agree to follow the project's code of conduct.
  • I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

  • Set GDAL_VCPKG environment variable to 1 to enable VCPKG support.
  • Set GDAL_VCPKG_TRIPLET to the desired triplet (e.g. x64-windows-static).
  • VCPKG_ROOT must point to the VCPKG root directory.
  • Install GDAL with vcpkg install gdal:x64-windows-static (or the desired triplet).
  • Install pkgconf with vcpkg install pkgconf:x64-windows-static.
  • Set PKG_CONFIG to the pkgconf executable (e.g. %VCPKG_ROOT%\installed\x64-windows-static\tools\pkgconf\pkgconf.exe).
  • Set PKG_CONFIG_PATH to %VCPKG_ROOT%\installed\x64-windows-static\lib\pkgconfig.
  • When building non-static version, copy missing dlls from %VCPKG_ROOT%\installed\x64-windows\bin to the executable directory.

@metasim
Copy link
Contributor

metasim commented Dec 20, 2023

@aljen Thanks for the PR! I'm not a windows (nor vcpkg) guy, so not sure how to review this. I'll reach out to the community in discord and see if anyone volunteers.

@aljen
Copy link
Author

aljen commented Dec 21, 2023

@aljen Thanks for the PR! I'm not a windows (nor vcpkg) guy, so not sure how to review this. I'll reach out to the community in discord and see if anyone volunteers.

No problem! :)
I could walk you through it, btw it's easier to follow if in 'files changed' you click gear icon and hide whitespace changes.
It's just an additional if, which if detected that we want to use gdal from VCPKG, it helps rust see libs from it, and in case of static variant, adds missing system's dlls dependencies :)

panic!("GDAL_VCPKG requires GDAL_VCPKG_TRIPLET to be set.");
}

let vcpkg_root = vcpkg_root.unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let vcpkg_root = vcpkg_root.unwrap();
let vcpkg_root = vcpkg_root.expect("GDAL_VCPKG requires VCPKG_ROOT to be set");

(same for vcpkg_triplet)

let pkg_config = env::var("PKG_CONFIG");
let pkg_config_path = env::var("PKG_CONFIG_PATH");

let required_pkg_config = vcpkg_install_dir
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand these. If we know the correct PKG_CONFIG, why does the user have to set it?

.unwrap()
.to_owned();

let valid_pkg_config = match pkg_config {
Copy link
Member

Choose a reason for hiding this comment

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

I think this would look a bit nicer like:

            if !matches!(pkg_config, Some(required_pkg_config)) {
                panic!("GDAL_VCPKG requires PKG_CONFIG to be set to '{required_pkg_config}'.");
            }

println!("cargo:rustc-link-search={}", lib_dir.to_str().unwrap());
println!("cargo:rustc-link-lib={link_type}={lib_name}");
Copy link
Member

Choose a reason for hiding this comment

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

👍

@@ -2,6 +2,10 @@

## Unreleased

- Added support for VCPKG. Set `GDAL_VCPKG` environment variable to `1` to enable VCPKG support. Set `GDAL_VCPKG_TRIPLET` to the desired triplet (e.g. `x64-windows-static`). `VCPKG_ROOT` must point to the VCPKG root directory. Install GDAL with `vcpkg install gdal:x64-windows-static` (or the desired triplet). Install pkgconf with `vcpkg install pkgconf:x64-windows-static`, set `PKG_CONFIG` to the pkgconf executable (e.g. `%VCPKG_ROOT%\installed\x64-windows-static\tools\pkgconf\pkgconf.exe`) and `PKG_CONFIG_PATH` to `%VCPKG_ROOT%\installed\x64-windows-static\lib\pkgconfig`. When building non-static version, copy missing dlls from `%VCPKG_ROOT%\installed\x64-windows\bin` to the executable directory.
Copy link
Member

Choose a reason for hiding this comment

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

Can you move the usage instructions into the gdal-sys README.md. That way, people might actually find them 🙂.

@metasim metasim mentioned this pull request Jan 5, 2024
8 tasks
@lnicola
Copy link
Member

lnicola commented Jan 5, 2024

Ping @aljen can you take another look at this?

@aljen
Copy link
Author

aljen commented Jan 9, 2024

Yeah, I planned to do this over the weekend, I had my plate full lately :)

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