rust-lang / rustc_codegen_gcc

libgccjit AOT codegen for rustc
Apache License 2.0
920 stars 60 forks source link

Building is difficult #269

Open cbeuw opened 1 year ago

cbeuw commented 1 year ago

I managed to build this repo, but I found the build process quite difficult, primarily due to the various shell scripts

  1. prepare.sh pulls hyperfine, rand, regex and simple-raytracer, none of which are required to build rustc_codegen_gcc or rust sysroot. These benchmark programs should be moved into a separate scripts
  2. prepare.sh calls cargo build, which shouldn't be done in the preparation stage. Additionally this is bound to fail as it doesn't know where to find libgccjit.so
  3. The CIs run prepare_build.sh instead, which has the right behaviour and is just prepare.sh if the above two steps are removed. Perhaps update README to replace mentions of prepare.sh with prepare_build.sh?
  4. README calls for LIBRARY_PATH AND LD_LIBRARY_PATH to be set while calling ./build.sh, but this is redundant as the script sets them https://github.com/rust-lang/rustc_codegen_gcc/blob/0cbf2b97157e0c3e2d1f809453a527bed4689247/build.sh#L44-L45
  5. build.sh, config.sh, and test.sh overwrites the whole LIBRARY_PATH and LD_LIBRARY_PATH, but the user environment may be non-empty (e.g. when under an environment managed by SPACK) . They should be appended instead.

These are all very easy to clean up, though you may have some other considerations so I thought I'd open an issue instead of a PR.

cbeuw commented 1 year ago

Additionally, I don't think it makes sense to copy and patch the sysroot (via prepare.sh, prepare_build.sh and eventually build_sysroot/prepare_sysroot_src.sh) before the codegen backend is built (which can be done simply with cargo build with the right LIBRARY_PATH set (LD_LIBRARY_PATH is not required). build_sysroot/prepare_sysroot_src.sh should be run just before building the sysroot

antoyo commented 1 year ago

Hi. Thanks for these suggestions. I'd be happy to accept a PR with those changes.