fix: use c++ 17

[winstxnhdw]

Closes #1190
Closes #1184
Closes #1178

[codecov]

Codecov Report

Attention: Patch coverage is 33.33333% with 14 lines in your changes missing coverage. Please review.

Project coverage is 46.93%. Comparing base (19654bf) to head (513caa0).

Files	Patch %	Lines
crates/engine_spx2html/src/fontfile.rs	0.00%	6 Missing ⚠️
crates/engine_spx2html/src/fonts.rs	0.00%	4 Missing ⚠️
crates/engine_spx2html/src/initialization.rs	0.00%	2 Missing ⚠️
crates/engine_spx2html/src/assets.rs	0.00%	1 Missing ⚠️
crates/xdv/src/lib.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1202      +/-   ##
==========================================
- Coverage   46.99%   46.93%   -0.06%     
==========================================
  Files         176      177       +1     
  Lines       65196    65191       -5     
==========================================
- Hits        30639    30599      -40     
- Misses      34557    34592      +35     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[CraftSpider]

Thank you for your work on this! I'll merge this once I find time to review it.

[winstxnhdw]

FWIW, it is working fine on my project.

[CraftSpider]

I'm not worried about the coverage failure personally - a decrease of .06 isn't really a big deal, and the clippy fixes I'm guessing just altered line hits slightly.

[inferiorhumanorgans]

This does not fix #1178 (regardless of whether or not external-harfbuzz is enabled).

[winstxnhdw]

This does not fix #1178 (regardless of whether or not external-harfbuzz is enabled).

Are you on MacOS?

[sjml]

For what it's worth, this built successfully for me. (I have not explored further or tried to use it; not much bandwidth at the moment. But it does look like the fixes in this repo let it build.) I'm on macOS, for whatever that's worth.

cargo new tectonic-embed
cd tectonic-embed
cargo add --git https://github.com/winstxnhdw/tectonic tectonic
PKG_CONFIG_PATH=$(brew --prefix icu4c)/lib/pkgconfig cargo build

[inferiorhumanorgans]

This does not fix #1178 (regardless of whether or not external-harfbuzz is enabled).

Are you on MacOS?

Yes. 14.3.

If I don't set PKG_CONFIG_PATH it can't/won't find the icu-uc files. If I do and use external-harfbuzz, the build fails because it can't find the headers, if I use the vendored harfbuzz it links against the system icu and spits out a bunch of undefined symbols.

[sjml]

This does not fix #1178 (regardless of whether or not external-harfbuzz is enabled).

Are you on MacOS?

Yes. 14.3.

If I don't set PKG_CONFIG_PATH it can't/won't find the icu-uc files. If I do and use external-harfbuzz, the build fails because it can't find the headers, if I use the vendored harfbuzz it links against the system icu and spits out a bunch of undefined symbols.

The PKG_CONFIG_PATH variable will probably always be needed on macOS builds because the system includes old headers that would get picked up otherwise.

The problem with external harfbuzz probably needs to be figured out, though, but it's at least separate from #1178. I was just verifying that this PR does in fact close that issue that I opened.

[inferiorhumanorgans]

Well, no. I literally described the exact problems #1178 describes. Those still persist with winstxnhdw/master.

[sjml]

Well, no. I literally described the exact problems #1178 describes. Those still persist with winstxnhdw/master.

Sorry, I must have misread your comment. I thought you were saying it didn't work without PKG_CONFIG_PATH, which is expected. When I ran the console commands from my earlier comment, though, it did build the project. Are you saying you get a different result from those same commands?

[winstxnhdw]

Well, no. I literally described the exact problems #1178 describes. Those still persist with winstxnhdw/master.

Strange. It’s fine on my end as well. Might want to check if it’s something else on your end.

[inferiorhumanorgans]

Well, no. I literally described the exact problems #1178 describes. Those still persist with winstxnhdw/master.

Strange. It’s fine on my end as well. Might want to check if it’s something else on your end.

Right back atcha. From looking at things, I'm not sure how it's actually building for anyone. E.g.

https://github.com/harfbuzz/harfbuzz/blob/main/src/harfbuzz.pc.in

You can see here that pkg-config file for harfbuzz includes the harfbuzz directory (e.g. /opt/homebrew/Cellar/harfbuzz/9.0.0/include/harfbuzz), but one of the errors stems from tectonic_xetex_layout.h looking for harfbuzz/hb.h.

[sjml]

Can you show the actual error that you get if you run these lines?

cargo new tectonic-embed
cd tectonic-embed
cargo add --git https://github.com/winstxnhdw/tectonic tectonic
PKG_CONFIG_PATH=$(brew --prefix icu4c)/lib/pkgconfig cargo build

[winstxnhdw]

Right back atcha. From looking at things, I'm not sure how it's actually building for anyone. E.g.

https://github.com/harfbuzz/harfbuzz/blob/main/src/harfbuzz.pc.in

You can see here that pkg-config file for harfbuzz includes the harfbuzz directory (e.g. /opt/homebrew/Cellar/harfbuzz/9.0.0/include/harfbuzz), but one of the errors stems from tectonic_xetex_layout.h looking for harfbuzz/hb.h.

Well, I have a Docker build that is able to build this successfully. If you can come up with a quick Dockerfile that exhibits your issue, we can look into it.

[inferiorhumanorgans]

Can you show the actual error that you get if you run these lines?

cargo new tectonic-embed
cd tectonic-embed
cargo add --git https://github.com/winstxnhdw/tectonic tectonic
PKG_CONFIG_PATH=$(brew --prefix icu4c)/lib/pkgconfig cargo build

There's no error because tectonic is not being use and as a result the linker doesn't bother resolving tectonic's symbols.

If I invoke something from tectonic, the linker blows up with undefined symbols. This ended up being a conflict with another crate.

If I enable the external-harfbuzz feature per #1178, the build fails as described.

[inferiorhumanorgans]

Right back atcha. From looking at things, I'm not sure how it's actually building for anyone. E.g.
https://github.com/harfbuzz/harfbuzz/blob/main/src/harfbuzz.pc.in
You can see here that pkg-config file for harfbuzz includes the harfbuzz directory (e.g. /opt/homebrew/Cellar/harfbuzz/9.0.0/include/harfbuzz), but one of the errors stems from tectonic_xetex_layout.h looking for harfbuzz/hb.h.

Well, I have a Docker build that is able to build this successfully. If you can come up with a quick Dockerfile that exhibits your issue, we can look into it.

Right, this is about mac builds with the external-harfbuzz feature enabled. On a Linux system (e.g. docker) harfbuzz is probably being installed into a path that the compiler is already searching e.g. /usr/include. Take a look at the harfbuzz pkg-config files I linked to above. They append the harfbuzz subdirectory to the compiler search path. Now look at how tectonic refers to the harfbuzz headers. Tectonic assumes that the harfbuzz subdirectory is not part of the compiler's search path.

This pull request doesn't fix the problems described in #1178.

[sjml]

If I use this main.rs file, it compiles and runs just fine provided I continue to give the appropriate PKG_CONFIG_PATH variable.

use tectonic;

fn main() {
    let latex = r#"
    \documentclass{article}
    \begin{document}
    Hello, world!
    \end{document}
    "#;

    let pdf_data: Vec<u8> = tectonic::latex_to_pdf(latex).expect("processing failed");
    println!("Output PDF size is {} bytes", pdf_data.len());
}

> PKG_CONFIG_PATH=$(brew --prefix icu4c)/lib/pkgconfig cargo build
[...]
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 33.64s

Running through cargo also needs the environment variable:

> PKG_CONFIG_PATH=$(brew --prefix icu4c)/lib/pkgconfig cargo run
[...]
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 20.07s
     Running `target/debug/tectonic-embed`
Output PDF size is 3031 bytes

But the created executable runs just fine:

> ./target/debug/tectonic-embed
Output PDF size is 3031 bytes

[CraftSpider]

Upon review, this looks good to me. I'm not able to speak to the PKG_CONFIG failures right now, but the changes in this are at least necessary to fix some issues, and this is a good prerequisite for #1209. As such, I'm going to merge, but feel free to re-open any issues that appear to still occur.