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

Revert "Update CI to use Android NDK r25b" #104628

Merged
merged 1 commit into from
Nov 21, 2022

Conversation

alex-pinkus
Copy link

@alex-pinkus alex-pinkus commented Nov 20, 2022

This reverts commit bf7f1ca (pull request #102332).

The relevant discussion can be found in #103673, where it was agreed that more time is needed to warn the community of the upcoming breakage.

This PR is for the master branch, where a conflict was recently introduced due to 6d81602. The conflict is in cc_detect.rs, where the code that corrects the target triple was moved to a new function called ndk_compiler(). This puts the old logic in the ndk_compiler function, and assumes that it works properly in the other location where that code is being called. I would appreciate review from @pietroalbini to understand how we can test that the reverted logic is also suitable for the additional use case (seems to be related to setting cc and cxx). I've confirmed already that with these changes I can compile for armv7-linux-androideabi, aarch64-linux-android, i686-linux-android, and x86_64-linux-android using x.py.

A separate revert for the beta branch will be required, since the original change has already made it to beta. The beta revert is available at alex-pinkus@3fa0d94, but I'm not sure of the process for staging that PR.

@rustbot
Copy link
Collaborator

rustbot commented Nov 20, 2022

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) soon.

Please see the contribution instructions for more information.

@rustbot rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Nov 20, 2022
@Mark-Simulacrum Mark-Simulacrum added beta-nominated Nominated for backporting to the compiler in the beta channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Nov 20, 2022
@Mark-Simulacrum
Copy link
Member

Thanks for preparing the beta commit. I'll pick that up into the beta backport rollup tomorrow (implicitly beta accepting this revert per #103673 (comment) and it being a revert).

@Mark-Simulacrum Mark-Simulacrum added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Nov 20, 2022
@alex-pinkus alex-pinkus marked this pull request as ready for review November 20, 2022 03:48
@Mark-Simulacrum Mark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Nov 20, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 20, 2022
…mulacrum

[beta] backport

*  Use nominal_obligations_without_const in wf for FnDef rust-lang#104180
*  Don't silently eat label before block in block-like expr rust-lang#103986
*  Revert "Update CI to use Android NDK r25b" rust-lang#104628
*  rustdoc: Resolve doc links in external traits having local impls rust-lang#104364
*  Use 64 bits for incremental cache in-file positions rust-lang#104164
*  rustdoc: Do not add external traits to the crate in register_res rust-lang#103649
*  Revert "Normalize opaques with escaping bound vars" rust-lang#103509
* Bump to released stable compiler
* [beta] Update cargo rust-lang#104486

r? `@Mark-Simulacrum`
@pietroalbini
Copy link
Member

This PR is for the master branch, where a conflict was recently introduced due to 6d81602. The conflict is in cc_detect.rs, where the code that corrects the target triple was moved to a new function called ndk_compiler(). This puts the old logic in the ndk_compiler function, and assumes that it works properly in the other location where that code is being called. I would appreciate review from @pietroalbini to understand how we can test that the reverted logic is also suitable for the additional use case (seems to be related to setting cc and cxx).

CI will ensure the change works for the other uses of ndk_compiler(), so there should be no additional testing needed on your end. For more context on the change, setting CC and CXX to the relevant NDK path is needed to run some tests, otherwise the test harness doesn't know how to compile native code. If this change somehow broke the test, CI will fail.

@bors r+ rollup=iffy

@bors
Copy link
Contributor

bors commented Nov 21, 2022

📌 Commit 6f1c7b2 has been approved by pietroalbini

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 21, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 21, 2022
…rade, r=pietroalbini

Revert "Update CI to use Android NDK r25b"

This reverts commit bf7f1ca (pull request rust-lang#102332).

The relevant discussion can be found in rust-lang#103673, where it was agreed that more time is needed to warn the community of the upcoming breakage.

This PR is for the `master` branch, where a conflict was recently introduced due to 6d81602. The conflict is in `cc_detect.rs`, where the code that corrects the target triple was moved to a new function called `ndk_compiler()`. This puts the old logic in the `ndk_compiler` function, and assumes that it works properly in the other location where that code is being called. I would appreciate review from `@pietroalbini` to understand how we can test that the reverted logic is also suitable for the additional use case (seems to be related to setting `cc` and `cxx`). I've confirmed already that with these changes I can compile for `armv7-linux-androideabi`, `aarch64-linux-android`, `i686-linux-android`, and `x86_64-linux-android` using `x.py`.

A separate revert for the `beta` branch will be required, since the original change has already made it to beta. The beta revert is available at alex-pinkus@3fa0d94, but I'm not sure of the process for staging that PR.
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 21, 2022
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#104420 (Fix doc example for `wrapping_abs`)
 - rust-lang#104499 (rustdoc JSON: Use `Function` everywhere and remove `Method`)
 - rust-lang#104500 (`rustc_ast`: remove `ref` patterns)
 - rust-lang#104511 (Mark functions created for `raw-dylib` on x86 with DllImport storage class)
 - rust-lang#104595 (Add `PolyExistentialPredicate` type alias)
 - rust-lang#104605 (deduplicate constant evaluation in cranelift backend)
 - rust-lang#104628 (Revert "Update CI to use Android NDK r25b")
 - rust-lang#104662 (Streamline deriving on packed structs.)
 - rust-lang#104667 (Revert formatting changes of a test)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5b92892 into rust-lang:master Nov 21, 2022
@rustbot rustbot added this to the 1.67.0 milestone Nov 21, 2022
@alex-pinkus alex-pinkus deleted the revert-android-ndk-upgrade branch November 21, 2022 18:56
gendx added a commit to gendx/android-rust-library that referenced this pull request Jan 5, 2023
This is necessary since upstream reverted compatibility to NDK <= 22: rust-lang/rust#104628.

See the discussion at rust-lang/rust#103673.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants