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

Augment error message when opening files for external tools #899

Merged
merged 9 commits into from
Jun 23, 2022

Conversation

matz-e
Copy link
Contributor

@matz-e matz-e commented May 13, 2022

I missed a publication file when compiling a very old document using
biber. The only error message I got was

error: not found

which isn't very helpful. I've tried to augment the error, now it shows

error: can't open path `pub.bib`: not found

Hope this is helpful.

I missed a publication file when compiling a very old document using
biber.  The only error message I got was

    error: not found

which isn't very helpful.  I've tried to augment the error, now it shows

    error: can't open path `pub.bib`: not found

Hope this is helpful.
@codecov
Copy link

codecov bot commented May 13, 2022

Codecov Report

Merging #899 (1493593) into master (c77ef78) will increase coverage by 0.04%.
The diff coverage is 47.05%.

@@            Coverage Diff             @@
##           master     #899      +/-   ##
==========================================
+ Coverage   45.66%   45.70%   +0.04%     
==========================================
  Files         147      147              
  Lines       61335    61342       +7     
==========================================
+ Hits        28006    28035      +29     
+ Misses      33329    33307      -22     
Impacted Files Coverage Δ
crates/bridge_core/src/lib.rs 65.72% <0.00%> (ø)
crates/bridge_core/support/support.c 79.10% <0.00%> (-1.20%) ⬇️
crates/bridge_flate/src/lib.rs 33.89% <ø> (ø)
crates/engine_bibtex/src/lib.rs 50.00% <ø> (-4.17%) ⬇️
crates/engine_xdvipdfmx/src/lib.rs 94.11% <ø> (ø)
crates/engine_xetex/src/lib.rs 82.75% <ø> (ø)
crates/pdf_io/pdf_io/dpx-pdfobj.c 43.37% <50.00%> (ø)
...ates/xetex_layout/layout/xetex-XeTeXFontMgr_FC.cpp 64.89% <60.00%> (-0.52%) ⬇️
src/driver.rs 74.40% <66.66%> (+0.52%) ⬆️
crates/engine_xdvipdfmx/xdvipdfmx/dvipdfmx.c 68.89% <100.00%> (+0.14%) ⬆️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c77ef78...1493593. Read the comment docs.

@pkgw
Copy link
Collaborator

pkgw commented May 17, 2022

Thanks so much! It looks like there's a build error associated with some new behavior on nightly Rust. If you have the chance to look into it, that would be great, but I'll plan to look into this and push a fix to your branch.

@pkgw
Copy link
Collaborator

pkgw commented May 17, 2022

As for the implementation, Tectonic has the macro ctry! ("contextualized try!") for providing a bit more context in error reporting output, building on the anyhow crate. Would you be able to use that instead of map_err? There are some examples of the usage in the file in question; the basic syntax is:

ctry!($fallible_expr; "format string {}", args...)

@matz-e
Copy link
Contributor Author

matz-e commented May 19, 2022

Thank you for the comments! I'll switch to the macro.

@matz-e
Copy link
Contributor Author

matz-e commented May 19, 2022

Thanks, much nicer that way. Now gives me the following:

error: can't open path `pub.bib`
caused by: not found

@pkgw
Copy link
Collaborator

pkgw commented May 20, 2022

D'oh, now it looks like the new release of Rust 1.61 has broken some of our cross-build infrastructure, which happens occasionally. I will hope to get that fixed up here, but I'm about to go offline for two weeks, and I may not be able to come up with a fix before that happens. So this PR may have to sit idle for a little while — the cross-build setup is very gnarly and I'm definitely not going to ask anyone else to try to understand what the heck is going on in its guts.

@matz-e
Copy link
Contributor Author

matz-e commented May 20, 2022

No worries, hope you can enjoy your time offline! Looks to me like a bunch of linking issues, but I'm also no expert on cross.

@pkgw
Copy link
Collaborator

pkgw commented Jun 9, 2022

I was hoping to take a look at this this week, but I've been tied up getting ready for a big astronomy conference that's next week, so I'm afraid I won't be able to do that for a little while longer ...

@pkgw
Copy link
Collaborator

pkgw commented Jun 11, 2022

Cool, I think I've identified the issue here. There was a linker behavior change introduced by:

rust-lang/rust#93901

That looks like it has broken the extra static libraries added here in our cross-builder images:

https://github.com/tectonic-typesetting/tectonic-ci-support/blob/master/cross-images/06_setup_env.sh#L24

Looking at the logs, these libraries are appearing on the linker command line well ahead of the static libraries like FreeType that depend on them. Ideally we would make sure that the extra deps appeared at the end of the link line, but it's not obvious to me how to make that happen. But the new +whole-archive static library linker option should restore the old behavior that worked.

I'll try creating some new cross-builder images, which should magically fix this problem if the diagnosis and solution are correct.

pkgw added 4 commits June 23, 2022 14:33
The latest version of Alpine Linux seems to provide a static ICU that no
longer has the "macintosh" converter built in. So don't error out if it
fails to load; just hope that everything will be OK.
As per rust-lang/libc#1848, we can't safely do this since the definition
of time_t depends on which version of Musl you're using on 32-bit
systems (the "time64" update for Musl 1.2), and this actually started
biting us on the arm-unknown-linux-musleabihf cross build.
Unfortunately, ISO C makes virtually no guarantees about what time_t is,
so I don't see a practical way to handle the different possibilities
robustly. We'll just cast from i64 or u64 to time_t in C and hope for
the best.
@pkgw
Copy link
Collaborator

pkgw commented Jun 23, 2022

I think I finally fixed the cross-build issues!

update: fixed two, broke one ...

@pkgw
Copy link
Collaborator

pkgw commented Jun 23, 2022

Finally!

@pkgw pkgw merged commit a1d1212 into tectonic-typesetting:master Jun 23, 2022
@matz-e
Copy link
Contributor Author

matz-e commented Jun 28, 2022

Thank you for the merge! Sorry, been very busy myself!

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.

2 participants