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
gnirehtet: 2.2.1 -> 2.3 #46735
gnirehtet: 2.2.1 -> 2.3 #46735
Conversation
No attempt on aarch64-linux (full log) The following builds were skipped because they don't evaluate on aarch64-linux: gnirehtet Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: gnirehtet Partial log (click to expand)
|
While testing this, I noticed one occurence of adb is missing from our patch. diff --git a/pkgs/tools/networking/gnirehtet/paths.patch b/pkgs/tools/networking/gnirehtet/paths.patch
index 72a8445d83b..600802cb57f 100644
--- a/pkgs/tools/networking/gnirehtet/paths.patch
+++ b/pkgs/tools/networking/gnirehtet/paths.patch
@@ -1,7 +1,7 @@
-diff --git a/relay-rust/src/main.rs b/relay-rust/src/main.rs
-index 4f1be53..96d2e78 100644
---- a/relay-rust/src/main.rs
-+++ b/relay-rust/src/main.rs
+Index: gnirehtet/relay-rust/src/main.rs
+===================================================================
+--- gnirehtet.orig/relay-rust/src/main.rs
++++ gnirehtet/relay-rust/src/main.rs
@@ -299,7 +299,7 @@ impl Command for RelayCommand {
fn cmd_install(serial: Option<&String>) -> Result<(), CommandExecutionError> {
@@ -11,7 +11,7 @@ index 4f1be53..96d2e78 100644
}
fn cmd_uninstall(serial: Option<&String>) -> Result<(), CommandExecutionError> {
-@@ -467,8 +467,8 @@ fn exec_adb<S: Into<String>>(
+@@ -464,8 +464,8 @@ fn exec_adb<S: Into<String>>(
args: Vec<S>,
) -> Result<(), CommandExecutionError> {
let adb_args = create_adb_args(serial, args);
@@ -22,7 +22,7 @@ index 4f1be53..96d2e78 100644
Ok(exit_status) => {
if exit_status.success() {
Ok(())
-@@ -490,8 +490,8 @@ fn must_install_client(serial: Option<&String>) -> Result<bool, CommandExecution
+@@ -487,8 +487,8 @@ fn must_install_client(serial: Option<&S
serial,
vec!["shell", "dumpsys", "package", "com.genymobile.gnirehtet"],
);
@@ -33,3 +33,16 @@ index 4f1be53..96d2e78 100644
Ok(output) => {
if output.status.success() {
// the "regex" crate makes the binary far bigger, so just parse the versionCode
+Index: gnirehtet/relay-rust/src/adb_monitor.rs
+===================================================================
+--- gnirehtet.orig/relay-rust/src/adb_monitor.rs
++++ gnirehtet/relay-rust/src/adb_monitor.rs
+@@ -206,7 +206,7 @@ impl AdbMonitor {
+
+ fn start_adb_daemon() -> bool {
+ info!(target: TAG, "Restarting adb daemon");
+- match process::Command::new("adb")
++ match process::Command::new("@adb@")
+ .args(&["start-server"])
+ .status() {
+ Ok(exit_status) => { Except this, looks good to me. Thanks :) |
Thank you! |
No attempt on aarch64-linux (full log) The following builds were skipped because they don't evaluate on aarch64-linux: gnirehtet Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: gnirehtet Partial log (click to expand)
|
fn start_adb_daemon() -> bool { | ||
info!(target: TAG, "Restarting adb daemon"); | ||
- match process::Command::new("adb") | ||
+ match process::Command::new("@adb@") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it not easier and less likely to break to use makeWrapper
instead and extend the PATH?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially, the reason I went for a patch was that we need the patch anyway to fix the path to the apk.
https://github.com/Genymobile/gnirehtet/releases/tag/v2.3
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)cc @symphorien