Plume front arm support #402

Scalone
mcrosson scala 7 commity/ów z plume-front-arm-support do master 5 lat temu
mcrosson skomentował(-a) 5 lat temu (Zmigrowane z github.com)

This PR addresses #394 and #400

Changes

  • Update Dockerfile to add necessary lld dependency on arm platforms via a helper script
  • Run the build/install of plume-front with the necessary linker=lld option on arm platforms via a helper script
  • Add rust-toolchain to Docker container along with Cargo files to ensure the right rust toolchain is selected for later steps

Builds Tested On

  • x86-64
  • arm64 (aarch64)
  • arm32 (armv7l)

Noteworthy items

  • The rust wasm packages do NOT include rustc-lld for arm
  • The lld binaries published by llvm.org will NOT build plume-front with linker=lld rust args
  • lld built from sources (what this patch does) on arm DOES build plume-front with linker=lld rust args
This PR addresses #394 and #400 Changes - Update Dockerfile to add necessary lld dependency on arm platforms via a helper script - Run the build/install of plume-front with the necessary linker=lld option on arm platforms via a helper script - Add rust-toolchain to Docker container along with Cargo files to ensure the right rust toolchain is selected for later steps Builds Tested On - x86-64 - arm64 (aarch64) - arm32 (armv7l) Noteworthy items - The rust wasm packages do NOT include rustc-lld for arm - The lld binaries published by llvm.org will NOT build plume-front with linker=lld rust args - lld built from sources (what this patch does) on arm DOES build plume-front with linker=lld rust args
codecov[bot] skomentował(-a) 5 lat temu (Zmigrowane z github.com)

Codecov Report

Merging #402 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #402   +/-   ##
=======================================
  Coverage   27.88%   27.88%           
=======================================
  Files          63       63           
  Lines        7252     7252           
=======================================
  Hits         2022     2022           
  Misses       5230     5230
# [Codecov](https://codecov.io/gh/Plume-org/Plume/pull/402?src=pr&el=h1) Report > Merging [#402](https://codecov.io/gh/Plume-org/Plume/pull/402?src=pr&el=desc) into [master](https://codecov.io/gh/Plume-org/Plume/commit/2896eb1705a766cc98935d64d090932aa617a0b1?src=pr&el=desc) will **not change** coverage. > The diff coverage is `n/a`. ```diff @@ Coverage Diff @@ ## master #402 +/- ## ======================================= Coverage 27.88% 27.88% ======================================= Files 63 63 Lines 7252 7252 ======================================= Hits 2022 2022 Misses 5230 5230 ```
trinity-1686a skomentował(-a) 5 lat temu
Właściciel

I'd prefer plume-front.sh and wasm-deps.sh to be put in the script folder, else it seems all good

I'd prefer `plume-front.sh` and `wasm-deps.sh` to be put in the `script` folder, else it seems all good
trinity-1686a zrecenzowano 5 lat temu
trinity-1686a skomentował(-a) 5 lat temu
Właściciel

(you don't actually need to save PATH, it's an environment variable as any other, a modification is only applied to this process and its child, not its parents)

(you don't actually need to save PATH, it's an environment variable as any other, a modification is only applied to this process and its child, not its parents)
mcrosson (Zmigrowane z github.com) zrecenzowano 5 lat temu
mcrosson (Zmigrowane z github.com) skomentował(-a) 5 lat temu

Good point, I'll update the script.

Good point, I'll update the script.
trinity-1686a zażądał(-a) zmian 5 lat temu
trinity-1686a zostawił komentarz
Właściciel

Builds Tested On

  • x86-64
  • arm64 (aarch64)
  • arm32 (armv7l)

Are you sure you don't have uncommitted changes? Compilation failed for me with x86-64, with the issue about scoping lint being experimental

>Builds Tested On > - x86-64 > - arm64 (aarch64) > - arm32 (armv7l) Are you sure you don't have uncommitted changes? Compilation failed for me with x86-64, with the issue about scoping lint being experimental
trinity-1686a skomentował(-a) 5 lat temu
Właściciel

Compilation failed because diesel_cli compilation depends on nightly and stable is used by default

 COPY Cargo.toml Cargo.lock rust-toolchain ./
Compilation failed because diesel_cli compilation depends on nightly and stable is used by default ```suggestion COPY Cargo.toml Cargo.lock rust-toolchain ./ ```
mcrosson (Zmigrowane z github.com) zrecenzowano 5 lat temu
mcrosson (Zmigrowane z github.com) skomentował(-a) 5 lat temu

I have #400 open for that ; I'll update per your changes above and include #400 in the main PR comments too.

I have #400 open for that ; I'll update per your changes above and include #400 in the main PR comments too.
trinity-1686a zatwierdza te zmiany 5 lat temu
trinity-1686a zrecenzowano 5 lat temu
trinity-1686a skomentował(-a) 5 lat temu
Właściciel

ho sorry, I through it was in the same issue

ho sorry, I through it was in the same issue
mcrosson (Zmigrowane z github.com) zrecenzowano 5 lat temu
mcrosson (Zmigrowane z github.com) skomentował(-a) 5 lat temu

No worries, this trick solved something that was bugging me and I'm happy to take care of two tickets with a single PR.

No worries, this trick solved something that was bugging me and I'm happy to take care of two tickets with a single PR.
vielmetti skomentował(-a) 5 lat temu (Zmigrowane z github.com)

The lld binaries published by llvm.org will NOT build plume-front with linker=lld rust args

@mcrosson do you recall what's missing with the llvm distributed lld binaries? I wonder if that's something that might be fixed upstream.

> The lld binaries published by llvm.org will NOT build plume-front with linker=lld rust args @mcrosson do you recall what's missing with the llvm distributed lld binaries? I wonder if that's something that might be fixed upstream.
mcrosson skomentował(-a) 5 lat temu (Zmigrowane z github.com)

@vielmetti IIRC the Rust packaging does not include arm32 or 64 as tier 1 for lld-wasm. I think it's on the WASM side? My memory is mildly hazy, there were a lot of tickets that I read and I'm still not familiar with the rust ecosystem.

@vielmetti IIRC the Rust packaging does not include arm32 or 64 as tier 1 for lld-wasm. I think it's on the WASM side? My memory is mildly hazy, there were a lot of tickets that I read and I'm still not familiar with the rust ecosystem.

Recenzenci

trinity-1686a zatwierdza te zmiany 5 lat temu
Pull Request został scalony jako dfa89e227a.
Możesz także zobaczyć instrukcje wiersza poleceń.

Krok 1:

Z repozytorium twojego projektu, sprawdź nową gałąź i przetestuj zmiany.
git checkout -b plume-front-arm-support master
git pull origin plume-front-arm-support

Krok 2:

Połącz zmiany i zaktualizuj na Forgejo.
git checkout master
git merge --no-ff plume-front-arm-support
git push origin master
Zaloguj się, aby dołączyć do tej rozmowy.
Brak recenzentów
Brak kamienia milowego
Brak przypisanych
Uczestnicy 2
Powiadomienia
Termin realizacji
Data realizacji jest niewłaściwa lub spoza zakresu. Użyj formatu 'yyyy-mm-dd'.

Brak ustawionego terminu realizacji.

Zależności

No dependencies set.

Reference: Plume/Plume#402
Ładowanie…
Nie ma jeszcze treści.