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

Compile error on Windows with Rust 1.70 #559

Closed
yutannihilation opened this issue Jun 5, 2023 · 13 comments · Fixed by extendr/rextendr#285
Closed

Compile error on Windows with Rust 1.70 #559

yutannihilation opened this issue Jun 5, 2023 · 13 comments · Fixed by extendr/rextendr#285

Comments

@yutannihilation
Copy link
Contributor

yutannihilation commented Jun 5, 2023

Compile error

(Originally found in #558 (comment))

If we try to compile R package using extendr with Rust 1.70, we'll see several errors like this:

...snip...\std\src\sys\windows/handle.rs:290: undefined reference to `NtWriteFile'

It seems this is because Rust 1.70 requires DLL ntdll. cf., We encountered similar issue before with Rust 1.57: #339

So, how did I come up with this DLL? I first checked the line 290 of std\src\sys\windows/handle.rs, which is referred in the error message above.

https://github.com/rust-lang/rust/blame/1.70.0/library/std/src/sys/windows/handle.rs#L290

This line actually contains the symbol NtWriteFile, which is defined here:

https://github.com/rust-lang/rust/blob/36aa75e44d5c6a83d9cb61eeaaadf82354307015/library/std/src/sys/windows/c.rs#L1269-L1341

I don't know what compat_fn! means, but it seems this should be somehow releated to ntdll (So, I still don't understand what's happening completely).

Warning

Even if we fix the error above, we'll see this warning.

 * checking whether package 'testpkg' can be installed ... [20s] WARNING
Found the following significant warnings:
  Warning: corrupt .drectve at end of def file

It seems this indicates some incompatibility between compilers.

https://stackoverflow.com/questions/25161814/warning-corrupt-drectve-at-end-of-def-file

There are two possibilities:

  1. LLVM (Rust's codegen backend) and Rtools' GCC are incompatible
  2. Rust's MSVC toolchain and GNU target are incompatible

I think 2. is unlikely. If it were true, things should break more badly. So I guess 1 is the cause. LLVM was upgraded to version 16 in Rust 1.70.

@eitsupi
Copy link
Contributor

eitsupi commented Jun 5, 2023

Excellent finding.
Currently the Windows runner of GitHub Actions has Rust 1.69 installed by default, will the next update to Rust 1.70 cause all builds on GitHub including R-universe to fail?
https://github.com/actions/runner-images/blob/win22/20230517.1/images/win/Windows2022-Readme.md

@yutannihilation
Copy link
Contributor Author

Oh, good catch. I believe so...

@eitsupi
Copy link
Contributor

eitsupi commented Jun 5, 2023

Oh, good catch. I believe so...

😨

@yutannihilation
Copy link
Contributor Author

@yutannihilation
Copy link
Contributor Author

Quick update. The warning disappears if we use binutils >= 2.40, and Rtools43 contains binutils 2.40. What's unfortunate is the setup-r action uses a bit outdated version of Rtools.

rust-lang/rust#112368 (comment)

The setup-r action can be fixed easily, and GHA will install the latest one by default soon because chocolatey specifies the latest version of Rtools. So, warnings on R 4.3 will be resolved sooner or later. However, probably we can't fix R 4.2, so that's a headache.

@Ilia-Kosenkov
Copy link
Member

And I presume Rtools 4.3 is not backward-compatible with R < 4.3?

@yutannihilation
Copy link
Contributor Author

Yes.

I posted a summary on R-package-devel. This might help to catch up.

https://stat.ethz.ch/pipermail/r-package-devel/2023q2/009229.html (This contains one mistake. "it's the case on GitHub Actions" is not true)

@Ilia-Kosenkov
Copy link
Member

Thank you @yutannihilation for taking the lead on this issue and finding the root cause! I hope we can properly fix it soon (I'd hate to pin rust to < 1.70 for R < 4.3)

@yutannihilation
Copy link
Contributor Author

@Ilia-Kosenkov
Thanks, but I'm afraid there's no "proper fix" and I think all we can do is ignoring warnings. #560 fixes the build failures in the way. Does this look good to you? Or, by "proper fix", do you have some different approach in your mind?

@yutannihilation
Copy link
Contributor Author

Btw,

I presume Rtools 4.3 is not backward-compatible with R < 4.3?

I believe it's safer to presume so, but I found R-universe thinks it's backward-compatible...

https://github.com/r-universe/r-rust/actions/runs/5213842120/jobs/9409449202#step:9:11

@Ilia-Kosenkov
Copy link
Member

Should we try out Rtools v 43 for R-oldrel?

@yutannihilation
Copy link
Contributor Author

Do you mean extendr will require users to use Rtools43 for R 4.2 just for avoiding the warnings that are likely harmless? It doesn't sound a good idea to me...

@Ilia-Kosenkov
Copy link
Member

I get your point, just exploring options. Let's keep it as-is for now.

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 a pull request may close this issue.

3 participants