Fix compile error on Windows with Rust 1.70

[yutannihilation]

Fix extendr/extendr#559

[CGMossa]

LGTM

I don't know how you figured this out though.. Absolutely amazing.

Also it is a windows only phenomenon.

[yutannihilation]

Thanks, I wrote some explanation on extendr/extendr#559.

But I'm yet to figure out what this indicates.

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

[yutannihilation]

It seems it 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.

rust-lang/rust#109474

The attempt to upgrade LLVM to version 16 failed once for some reason. We may be able to find some context in this lengthy thread (I mean, I don't read this yet...).

rust-lang/rust#107224

[yutannihilation]

We may be able to find some context in this lengthy thread

Hmm, no. rust-lang/rust#109474 says it was "ABI-incompatibility between libstdc++ 7 and 8," so it seems unrelated.

[yutannihilation]

This one?

llvm/llvm-project@c5b3de6

[yutannihilation]

Minimal reproducible example

https://github.com/yutannihilation/rust170_gnu_warning

[Ilia-Kosenkov]

Looks like an LLVM thing that Yutani linked above. Should we raid llvm issues and ask there?

[yutannihilation]

Thanks, I think this is primarily Rust's issue. I'll report this to Rust's repo after waiting for a while to get answers in the user forum.

https://users.rust-lang.org/t/corrupt-drectve-warning-on-x86-64-pc-windows-gnu-target-with-rust-1-70/94912

I'd appreciate if you ask to LLVM's side!

Note that, I'm afraid this warning won't be resolved because this is what we can just ignore, no real harm, iiuc. But it probably means Rust package will be kicked out from CRAN...

[eitsupi]

But it probably means Rust package will be kicked out from CRAN...

Hopefully the version will be skipped as Rust on Windows in CRAN does not seem to be updated frequently......

[yutannihilation]

Yeah, but Jeroen said CRAN already used Rust 1.69, which was the latest at the moment, so I'm not sure if we can be that optimistic.

#279 (comment)

[yutannihilation]

Reported: rust-lang/rust#112368

[CGMossa]

Yep.

[yutannihilation]

@CGMossa @Ilia-Kosenkov
I'm merging this while this doesn't actually "fix" the warning on oldrel because I'm afraid there's no way to fix it actually. Please propose alternatives if you come up with a good idea.

[eitsupi]

Since this change seems important, could you please do a new patch release?
It has been over a week since the last release so it should be allowed......

[Ilia-Kosenkov]

Is this change so important? We hard-coded a dependency which is only needed if you run Rust 1.70 during compilation.
@yutannihilation, @CGMossa, what do you think?

[yutannihilation]

I think it would be frustrating especially for new users that their first R package using rextendr::use_extendr() won't work. That said, existing users won't be saved anyway, and ... probably a week is too frequent? CRAN Repository Policy says

Submitting updates should be done responsibly and with respect for the volunteers’ time. Once a package is established (which may take several rounds), “no more than every 1–2 months” seems appropriate.

[yutannihilation]

Is this change so important?

To be clear, I think this compilation error is a critical problem. That's why I tried so hard to investigate the cause and find the fix as soon as possible. What I'm not sure is how important this rextendr's template is. I use my own version of Makevars.win etc rather than the ones rextendr provides, so I think I'm not the right person to decide.

[CGMossa]

I simply hate any friction that may occur with fresh new users. While I think we should issue a patch, it could be that we should give it a week or so, just go be on the safe side.

But we cannot have a an issue for those who install latest r, latest rtools, latest rust..

[eitsupi]

probably a week is too frequent?

Just for information, I have been told that R actually checks to see if it is within a week of the last release and if there have been less than 6 releases in the past 6 months.

[yutannihilation]

Oh, I didn't know this, thanks.

[Ilia-Kosenkov]

Funny that in the modern era of CI CD you have 6 releases per year on CRAN. I suggest we wait a week and I will make a patch release with a comment that we updated templates to align with latest rust release

[eitsupi]

@Ilia-Kosenkov Now that a week has passed, perhaps a new release can be made? Thanks.

[eitsupi]

Since Rust on GitHub Actions is currently 1.70 by default, this problem occurs.

[yutannihilation]

@eitsupi
Just curious. Why is the CRAN release important to you? You need to fix the Makevars.win in your existing packages by yourself anyway. In particular, I'm wondering why you haven't fix prqlr yet. Is it related to how you set up GHA CI?

[eitsupi]

Just curious. Why does the CRAN release important to you? You need to fix the Makevars.win in your existing packages by yourself anyway. In particular, I'm wondering why you haven't fix prqlr yet. Is it related to how you set up GHA CI?

Simply because I myself suffered a lot from extendr's (at the time) existing templates not working.

[yutannihilation]

I see. I agree it's really frustrating.

[Ilia-Kosenkov]

I will do this on Monday if that is OK for you.