Fix external tool file copy to include sub directories, fix #843

[KevoSoftworks]

Fixes files used for external tools not being copied if they are located in sub directories (#843). This is done by checking the file directory against the temp directory. If they do not match, std::fs::create_dir_all() is used to create the directory structure for the file.

I would love to get some feedback on this implementation. Is it a decent implementation?, is it "rust-like"?, what can I change?, et cetera. It is my first time writing some rust, so I have no clue ;).

[codecov]

Codecov Report

Merging #845 (8c52dc2) into master (09b1575) will increase coverage by 0.00%.
The diff coverage is 71.42%.

@@           Coverage Diff           @@
##           master     #845   +/-   ##
=======================================
  Coverage   47.07%   47.08%           
=======================================
  Files         146      146           
  Lines       59493    59500    +7     
=======================================
+ Hits        28008    28014    +6     
- Misses      31485    31486    +1     

Impacted Files	Coverage Δ
crates/io_base/src/filesystem.rs	57.01% <ø> (ø)
src/driver.rs	73.31% <71.42%> (+0.10%)	⬆️
crates/bundles/src/zip.rs	0.00% <0.00%> (-3.71%)	⬇️
crates/io_base/src/lib.rs	76.27% <0.00%> (+0.42%)	⬆️

---

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 09b1575...8c52dc2. Read the comment docs.

[pkgw]

Thanks for your submission! I'm always glad to see people be willing to try writing their own Rust code for the first time — I appreciate that there's a learning curve to it!

In terms of the Rust style, your code looks great. I can think of a couple of things to check but they're not specific to Rust:

• Tectonic's TeX paths always use / as a directory separator, but the host OS doesn't necessarily do this. Will the path join yield the correct on-disk directory structure on Windows?
• The existing implementation should already be careful about this, but it would be good to double-check that if someone references a TeX path like ../../../../../../../../../../etc/passwd, Tectonic doesn't try to touch the filesystem outside of its temporary work directory.
• If you're going to call a function like create_dirs_all, there's no point in calling exists() separately — you might as well just create the directory unconditionally. In some cases the current pattern can lead to rare race condition issues, although in this case I don't think that can plausibly happen.

There are several things flagged by the CI:

• Codecov points out that the new code doesn't have test coverage. It would be great if you could add a test case that covered the new functionality, especially since this would address the issue about directory separators I mentioned above
• Clippy has some new complaints about existing code unrelated to your change. It looks like these are easy to fix so if you wouldn't mind making its recommended updates, that issue can be resolved
• The Windows CIs are failing on a test that is also unrelated to your change. I'm not quite sure where it's coming from — I'll need to look into it. It's not this PR's responsibility to fix this issue.

[KevoSoftworks]

* The existing implementation should already be careful about this, but it would be good to double-check that if someone references a TeX path like `../../../../../../../../../../etc/passwd`, Tectonic doesn't try to touch the filesystem outside of its temporary work directory.

Finding a solution to this without limiting the implementation proves to be a bit of a challenge. Technically, I could disallow or drop any relative parent (i.e. ../). However, this breaks when bibliographies are located in parent directories compared to the main tex file. This problem is compounded by the fact that the .bcf file refers to the given relative directories, for example <bcf:datasource type="file" datatype="bibtex">../main.bib</bcf:datasource>. Since biber uses these entries to create its output, I hit a roadblock. Even if I ensure files stay within the temporary directory, biber will not look for them in the correct place. I see a few solutions:

• Create a directory structure within the temporary directory that mimics the relative paths of the LaTeX project. In the example above, the working dir could be /tmp/rndstr1/rndstr2/, and the bib file located at /tmp/rndstr1/main.bib.
• Modify the datasource entries in the .bcf file to correspond to the files in the temporary directory. At this moment this would go quite over my head to be honest.
• Drop parent relative paths. This would probably be easiest to implement, but is also most limiting.

Maybe there exists some parameter for biber to help in this process, but I haven't looked in the documentation yet.

What do you think is the best course of action?

[KevoSoftworks]

At this point, I have implemented the last option. If a relative path references ../ an error will be thrown.

I'll have to check if this works properly on Windows too, but I need to setup an environment for that first.

[KevoSoftworks]

Seems I need to fix my formatting. Clippy is also complaining again about something unrelated. Should I go and fix that too?

[pkgw]

If you don't mind, for this latest Clippy issue I think it would be best to add an #[allow(clippy::if_same_then_else)] annotation to the function in question.

As for the formatting, yes, the CI requires that all code match the standard cargo fmt layout. I've found it helpful to turn on the "run code formatter on save" option in my various editors to make sure that I never forget to keep things in the standard format.

I hope to take another look at your code changes soon but several work things have come up that will probably keep me pretty busy for a little while, so I'm afraid I may not be as prompt as I'd like to follow up.

[KevoSoftworks]

Wow, cargo fmt is a very useful tool. It fixed the formatting for me automatically, instead of just complaining. Thank you for showing me!

I added the annotation and ran the same cargo clippy command as in the CI implementation, and it succeeded. So hopefully its CI will now succeed.

I will still be working on the same project for the coming few weeks, so it is no problem for me if the follow-up takes a while.

[pkgw]

This looks great now! Because the Windows builds are busted, the usual continuous deployment processes won't run when I merge this, but I'll work on getting those fixed up ASAP.

Thanks so much for your contribution!

[KevoSoftworks]

Awesome! I'm glad I could contribute to this project!

Also, many thanks for your guidance and input! It has been a great learning experience.