1495823 - The shell scripts to build Rust projects are duplicating code

Status: RESOLVED DUPLICATE of bug 1806960

(Firefox Build System :: Toolchains, enhancement)

Description

[Marco Castelluccio [:marco]]

taskcluster/scripts/misc/build-rust-size.sh, taskcluster/scripts/misc/build-sccache.sh, taskcluster/scripts/misc/build-grcov.sh have a lot of things in common. We should deduplicate the code between the three scripts.

Comment 1

[Gustavo Cesar [:glcesar]]

The scripts build-rust-size.sh and build-grcov.sh are doing very similar tasks indeed. However, build-rust-size.sh saves the parent dir ($PROJECT) while build-grcov.sh doesn't. How can I verify/test the impact if I reuse build-grcov.sh for build-rust-size.sh?

Comment 2

[Nathan Froyd [:froydnj]]

(In reply to Gustavo Cesar [:glcesar] from comment #1)

The scripts build-rust-size.sh and build-grcov.sh are doing very similar tasks indeed. However, build-rust-size.sh saves the parent dir ($PROJECT) while build-grcov.sh doesn't. How can I verify/test the impact if I reuse build-grcov.sh for build-rust-size.sh?

You can push your patch to try using ./mach try fuzzy --full and select all of the the grcov and rust-size toolchain tasks. If those build successfully, then your patch works! :)

Comment 3

[Marco Castelluccio [:marco]]

(In reply to Gustavo Cesar [:glcesar] from comment #1)

The scripts build-rust-size.sh and build-grcov.sh are doing very similar tasks indeed. However, build-rust-size.sh saves the parent dir ($PROJECT) while build-grcov.sh doesn't. How can I verify/test the impact if I reuse build-grcov.sh for build-rust-size.sh?

What Nathan said ;)

Note, there might also be other build-* scripts in the same directory sharing similar code.

Comment 4

[Gustavo Cesar [:glcesar]]

Hi,

Sorry for the delay. I'm trying to get commit access level 1. Can some of you give me a voucher on Bug 1598116, so I can use the command you have given?

I'm attaching my patch, so you can take a look and give any feedback.

Comment 5

[Gustavo Cesar [:glcesar]]

Attachment: build-rust-refactor.diff

Comment 6

[Nathan Froyd [:froydnj]]

Comment on attachment 9112104 [details] [diff] [review]
build-rust-refactor.diff

Review of attachment 9112104 [details] [diff] [review]:
-----------------------------------------------------------------

This is useful, but I think the idea was that for, e.g. `build-cbindgen.sh` and `build-geckodriver.sh`, those scripts get reduced to something like:

```
#!/bin/bash

. /path/to/cargo_build_deploy.sh

build_cargo_project cbindgen
```

and all the logic for setting up paths, programs, arguments to cargo, etc. lives in `/path/to/cargo_build_deploy.sh`.  This change is a first step, but I think we can go farther.  It may be difficult to factor out common pieces for `build-sccache.sh` or similar files, though.

::: taskcluster/scripts/misc/build-utils.sh
@@ +1,4 @@
> +#!/bin/bash
> +set -x -e -v
> +
> +# Utilities functions that are shared among multiple misc scripts

Nit: "Utility functions..."

@@ +27,5 @@
> +    esac
> +}
> +
> +function export_rustc_to_path {
> +    export PATH=$MOZ_FETCHES_DIR/rustc/bin:$PATH

This function is different from the code it's replacing in two ways: a) it `export`'s `PATH` and b) it doesn't use the `cd` trick of the other code (which I believe is necessary on Windows).  Please change at least the latter.

Comment 7

[Nicholas Nethercote [inactive]]

Note that bug 1604964 is going to reduplicate this code again, for fix-stacks.