-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Build GCC on CI #136921
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
Build GCC on CI #136921
Changes from 5 commits
87fbd4e
a0cf0f2
17472a9
b4d9d02
58be00a
ba7d5d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -9,13 +9,13 @@ | |||
//! ensure that they're always in place if needed. | ||||
|
||||
use std::fs; | ||||
use std::path::PathBuf; | ||||
use std::path::{Path, PathBuf}; | ||||
use std::sync::OnceLock; | ||||
|
||||
use build_helper::ci::CiEnv; | ||||
|
||||
use crate::Kind; | ||||
use crate::core::builder::{Builder, RunConfig, ShouldRun, Step}; | ||||
use crate::core::builder::{Builder, Cargo, RunConfig, ShouldRun, Step}; | ||||
use crate::core::config::TargetSelection; | ||||
use crate::utils::build_stamp::{BuildStamp, generate_smart_stamp_hash}; | ||||
use crate::utils::exec::command; | ||||
|
@@ -29,7 +29,8 @@ pub struct Meta { | |||
} | ||||
|
||||
pub enum GccBuildStatus { | ||||
AlreadyBuilt, | ||||
/// libgccjit is already built at this path | ||||
AlreadyBuilt(PathBuf), | ||||
ShouldBuild(Meta), | ||||
} | ||||
|
||||
|
@@ -41,9 +42,6 @@ pub fn prebuilt_gcc_config(builder: &Builder<'_>, target: TargetSelection) -> Gc | |||
// Initialize the gcc submodule if not initialized already. | ||||
builder.config.update_submodule("src/gcc"); | ||||
|
||||
// FIXME (GuillaumeGomez): To be done once gccjit has been built in the CI. | ||||
// builder.config.maybe_download_ci_gcc(); | ||||
|
||||
let root = builder.src.join("src/gcc"); | ||||
let out_dir = builder.gcc_out(target).join("build"); | ||||
let install_dir = builder.gcc_out(target).join("install"); | ||||
|
@@ -70,19 +68,37 @@ pub fn prebuilt_gcc_config(builder: &Builder<'_>, target: TargetSelection) -> Gc | |||
stamp.path().display() | ||||
)); | ||||
} | ||||
return GccBuildStatus::AlreadyBuilt; | ||||
let path = libgccjit_built_path(&install_dir); | ||||
if path.is_file() { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I usually prefer explicitly marking that it should be a file, but no strong opinion on this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am okay with it. It just seemed weird to me. |
||||
return GccBuildStatus::AlreadyBuilt(path); | ||||
} else { | ||||
builder.info(&format!( | ||||
"GCC stamp is up-to-date, but the libgccjit.so file was not found at `{}`", | ||||
path.display(), | ||||
)); | ||||
} | ||||
} | ||||
|
||||
GccBuildStatus::ShouldBuild(Meta { stamp, out_dir, install_dir, root }) | ||||
} | ||||
|
||||
/// Returns the path to a libgccjit.so file in the install directory of GCC. | ||||
fn libgccjit_built_path(install_dir: &Path) -> PathBuf { | ||||
install_dir.join("lib/libgccjit.so") | ||||
} | ||||
|
||||
#[derive(Debug, Clone, Hash, PartialEq, Eq)] | ||||
pub struct Gcc { | ||||
pub target: TargetSelection, | ||||
} | ||||
|
||||
#[derive(Clone)] | ||||
pub struct GccOutput { | ||||
pub libgccjit: PathBuf, | ||||
} | ||||
|
||||
impl Step for Gcc { | ||||
type Output = bool; | ||||
type Output = GccOutput; | ||||
|
||||
const ONLY_HOSTS: bool = true; | ||||
|
||||
|
@@ -94,14 +110,14 @@ impl Step for Gcc { | |||
run.builder.ensure(Gcc { target: run.target }); | ||||
} | ||||
|
||||
/// Compile GCC for `target`. | ||||
fn run(self, builder: &Builder<'_>) -> bool { | ||||
/// Compile GCC (specifically `libgccjit`) for `target`. | ||||
fn run(self, builder: &Builder<'_>) -> Self::Output { | ||||
let target = self.target; | ||||
|
||||
// If GCC has already been built, we avoid building it again. | ||||
let Meta { stamp, out_dir, install_dir, root } = match prebuilt_gcc_config(builder, target) | ||||
{ | ||||
GccBuildStatus::AlreadyBuilt => return true, | ||||
GccBuildStatus::AlreadyBuilt(path) => return GccOutput { libgccjit: path }, | ||||
GccBuildStatus::ShouldBuild(m) => m, | ||||
}; | ||||
|
||||
|
@@ -110,8 +126,9 @@ impl Step for Gcc { | |||
let _time = helpers::timeit(builder); | ||||
t!(fs::create_dir_all(&out_dir)); | ||||
|
||||
let libgccjit_path = libgccjit_built_path(&install_dir); | ||||
if builder.config.dry_run() { | ||||
return true; | ||||
return GccOutput { libgccjit: libgccjit_path }; | ||||
Comment on lines
+129
to
+131
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can make There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't like when Steps return just PathBuf, it's not obvious what does the path represent. In the future the struct might contain other attributes, the path to the libgccjit file isn't exactly the same as the built output of thw GCC step. |
||||
} | ||||
|
||||
// GCC creates files (e.g. symlinks to the downloaded dependencies) | ||||
|
@@ -173,11 +190,17 @@ impl Step for Gcc { | |||
|
||||
let lib_alias = install_dir.join("lib/libgccjit.so.0"); | ||||
if !lib_alias.exists() { | ||||
t!(builder.symlink_file(install_dir.join("lib/libgccjit.so"), lib_alias,)); | ||||
t!(builder.symlink_file(&libgccjit_path, lib_alias)); | ||||
} | ||||
|
||||
t!(stamp.write()); | ||||
|
||||
true | ||||
GccOutput { libgccjit: libgccjit_path } | ||||
} | ||||
} | ||||
|
||||
/// Configures a Cargo invocation so that it can build the GCC codegen backend. | ||||
pub fn add_cg_gcc_cargo_flags(cargo: &mut Cargo, gcc: &GccOutput) { | ||||
// Add the path to libgccjit.so to the linker search paths. | ||||
cargo.rustflag(&format!("-L{}", gcc.libgccjit.parent().unwrap().display())); | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We shouldn't prefer There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How else to pass the Path then? 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, will change. I'm pretty sure that we use
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should update them too (you don't have to do that in this PR). |
||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ FROM ubuntu:24.04 | |
ARG DEBIAN_FRONTEND=noninteractive | ||
|
||
RUN apt-get update && apt-get install -y --no-install-recommends \ | ||
bzip2 \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Q: Why is it required only in this image but not in others? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently we build and test GCC only on this one job, in PR builds, unless I missed something. |
||
g++ \ | ||
gcc-multilib \ | ||
make \ | ||
|
@@ -55,9 +56,6 @@ ENV RUST_CONFIGURE_ARGS \ | |
--set rust.thin-lto-import-instr-limit=10 | ||
|
||
COPY scripts/shared.sh /scripts/ | ||
COPY scripts/build-gccjit.sh /scripts/ | ||
|
||
RUN /scripts/build-gccjit.sh /scripts | ||
|
||
ARG SCRIPT_ARG | ||
|
||
|
This file was deleted.
Uh oh!
There was an error while loading. Please reload this page.