Skip to content

Commit

Permalink
fix: Fix build scripts not being rebuilt in some occasions
Browse files Browse the repository at this point in the history
  • Loading branch information
Veykril committed Jan 4, 2024
1 parent 9279c65 commit 443fc29
Show file tree
Hide file tree
Showing 15 changed files with 102 additions and 70 deletions.
2 changes: 1 addition & 1 deletion crates/hir-def/src/nameres/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, def_map: DefMap, tree_id: TreeI
};
(
name.as_name(),
CustomProcMacroExpander::new(hir_expand::proc_macro::ProcMacroId(
CustomProcMacroExpander::new(hir_expand::proc_macro::ProcMacroId::new(
idx as u32,
)),
)
Expand Down
4 changes: 2 additions & 2 deletions crates/hir-expand/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,8 @@ pub enum MacroCallKind {
},
Attr {
ast_id: AstId<ast::Item>,
// FIXME: This is being interned, subtrees can very quickly differ just slightly causing
// leakage problems here
// FIXME: This shouldn't be here, we can derive this from `invoc_attr_index`
// but we need to fix the `cfg_attr` handling first.
attr_args: Option<Arc<tt::Subtree>>,
/// Syntactical index of the invoking `#[attribute]`.
///
Expand Down
8 changes: 7 additions & 1 deletion crates/hir-expand/src/proc_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,13 @@ use syntax::SmolStr;
use crate::{db::ExpandDatabase, tt, ExpandError, ExpandResult};

#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub struct ProcMacroId(pub u32);
pub struct ProcMacroId(u32);

impl ProcMacroId {
pub fn new(u32: u32) -> Self {
ProcMacroId(u32)
}
}

#[derive(Copy, Clone, Eq, PartialEq, Debug, Hash)]
pub enum ProcMacroKind {
Expand Down
8 changes: 6 additions & 2 deletions crates/project-model/src/build_scripts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use serde::Deserialize;

use crate::{
cfg_flag::CfgFlag, utf8_stdout, CargoConfig, CargoFeatures, CargoWorkspace, InvocationLocation,
InvocationStrategy, Package,
InvocationStrategy, Package, TargetKind,
};

#[derive(Debug, Default, Clone, PartialEq, Eq)]
Expand Down Expand Up @@ -445,7 +445,11 @@ impl WorkspaceBuildScripts {
.collect();
for p in rustc.packages() {
let package = &rustc[p];
if package.targets.iter().any(|&it| rustc[it].is_proc_macro) {
if package
.targets
.iter()
.any(|&it| matches!(rustc[it].kind, TargetKind::Lib { is_proc_macro: true }))
{
if let Some((_, path)) = proc_macro_dylibs
.iter()
.find(|(name, _)| *name.trim_start_matches("lib") == package.name)
Expand Down
12 changes: 6 additions & 6 deletions crates/project-model/src/cargo_workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,6 @@ pub struct TargetData {
pub root: AbsPathBuf,
/// Kind of target
pub kind: TargetKind,
/// Is this target a proc-macro
pub is_proc_macro: bool,
/// Required features of the target without which it won't build
pub required_features: Vec<String>,
}
Expand All @@ -196,7 +194,10 @@ pub struct TargetData {
pub enum TargetKind {
Bin,
/// Any kind of Cargo lib crate-type (dylib, rlib, proc-macro, ...).
Lib,
Lib {
/// Is this target a proc-macro
is_proc_macro: bool,
},
Example,
Test,
Bench,
Expand All @@ -213,8 +214,8 @@ impl TargetKind {
"bench" => TargetKind::Bench,
"example" => TargetKind::Example,
"custom-build" => TargetKind::BuildScript,
"proc-macro" => TargetKind::Lib,
_ if kind.contains("lib") => TargetKind::Lib,
"proc-macro" => TargetKind::Lib { is_proc_macro: true },
_ if kind.contains("lib") => TargetKind::Lib { is_proc_macro: false },
_ => continue,
};
}
Expand Down Expand Up @@ -366,7 +367,6 @@ impl CargoWorkspace {
name,
root: AbsPathBuf::assert(src_path.into()),
kind: TargetKind::new(&kind),
is_proc_macro: &*kind == ["proc-macro"],
required_features,
});
pkg_data.targets.push(tgt);
Expand Down
38 changes: 19 additions & 19 deletions crates/project-model/src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ impl ProjectWorkspace {
let extra_targets = cargo[pkg]
.targets
.iter()
.filter(|&&tgt| cargo[tgt].kind == TargetKind::Lib)
.filter(|&&tgt| matches!(cargo[tgt].kind, TargetKind::Lib { .. }))
.filter_map(|&tgt| cargo[tgt].root.parent())
.map(|tgt| tgt.normalize().to_path_buf())
.filter(|path| !path.starts_with(&pkg_root));
Expand Down Expand Up @@ -912,17 +912,17 @@ fn cargo_to_crate_graph(

let mut lib_tgt = None;
for &tgt in cargo[pkg].targets.iter() {
if cargo[tgt].kind != TargetKind::Lib && !cargo[pkg].is_member {
if !matches!(cargo[tgt].kind, TargetKind::Lib { .. }) && !cargo[pkg].is_member {
// For non-workspace-members, Cargo does not resolve dev-dependencies, so we don't
// add any targets except the library target, since those will not work correctly if
// they use dev-dependencies.
// In fact, they can break quite badly if multiple client workspaces get merged:
// https://github.com/rust-lang/rust-analyzer/issues/11300
continue;
}
let &TargetData { ref name, kind, is_proc_macro, ref root, .. } = &cargo[tgt];
let &TargetData { ref name, kind, ref root, .. } = &cargo[tgt];

if kind == TargetKind::Lib
if matches!(kind, TargetKind::Lib { .. })
&& sysroot.map_or(false, |sysroot| root.starts_with(sysroot.src_root()))
{
if let Some(&(_, crate_id, _)) =
Expand All @@ -947,19 +947,24 @@ fn cargo_to_crate_graph(
cfg_options.clone(),
file_id,
name,
is_proc_macro,
kind,
target_layout.clone(),
false,
toolchain.cloned(),
);
if kind == TargetKind::Lib {
if let TargetKind::Lib { .. } = kind {
lib_tgt = Some((crate_id, name.clone()));
pkg_to_lib_crate.insert(pkg, crate_id);
}
// Even crates that don't set proc-macro = true are allowed to depend on proc_macro
// (just none of the APIs work when called outside of a proc macro).
if let Some(proc_macro) = libproc_macro {
add_proc_macro_dep(crate_graph, crate_id, proc_macro, is_proc_macro);
add_proc_macro_dep(
crate_graph,
crate_id,
proc_macro,
matches!(kind, TargetKind::Lib { is_proc_macro: true }),
);
}

pkg_crates.entry(pkg).or_insert_with(Vec::new).push((crate_id, kind));
Expand Down Expand Up @@ -1157,9 +1162,9 @@ fn handle_rustc_crates(
};

for &tgt in rustc_workspace[pkg].targets.iter() {
if rustc_workspace[tgt].kind != TargetKind::Lib {
let kind @ TargetKind::Lib { is_proc_macro } = rustc_workspace[tgt].kind else {
continue;
}
};
if let Some(file_id) = load(&rustc_workspace[tgt].root) {
let crate_id = add_target_crate_root(
crate_graph,
Expand All @@ -1169,7 +1174,7 @@ fn handle_rustc_crates(
cfg_options.clone(),
file_id,
&rustc_workspace[tgt].name,
rustc_workspace[tgt].is_proc_macro,
kind,
target_layout.clone(),
true,
toolchain.cloned(),
Expand All @@ -1178,12 +1183,7 @@ fn handle_rustc_crates(
// Add dependencies on core / std / alloc for this crate
public_deps.add_to_crate_graph(crate_graph, crate_id);
if let Some(proc_macro) = libproc_macro {
add_proc_macro_dep(
crate_graph,
crate_id,
proc_macro,
rustc_workspace[tgt].is_proc_macro,
);
add_proc_macro_dep(crate_graph, crate_id, proc_macro, is_proc_macro);
}
rustc_pkg_crates.entry(pkg).or_insert_with(Vec::new).push(crate_id);
}
Expand Down Expand Up @@ -1245,7 +1245,7 @@ fn add_target_crate_root(
cfg_options: CfgOptions,
file_id: FileId,
cargo_name: &str,
is_proc_macro: bool,
kind: TargetKind,
target_layout: TargetLayoutLoadResult,
rustc_crate: bool,
toolchain: Option<Version>,
Expand Down Expand Up @@ -1295,7 +1295,7 @@ fn add_target_crate_root(
cfg_options,
potential_cfg_options,
env,
is_proc_macro,
matches!(kind, TargetKind::Lib { is_proc_macro: true }),
if rustc_crate {
CrateOrigin::Rustc { name: pkg.name.clone() }
} else if pkg.is_member {
Expand All @@ -1306,7 +1306,7 @@ fn add_target_crate_root(
target_layout,
toolchain,
);
if is_proc_macro {
if let TargetKind::Lib { is_proc_macro: true } = kind {
let proc_macro = match build_data.as_ref().map(|it| it.proc_macro_dylib_path.as_ref()) {
Some(it) => it.cloned().map(|path| Ok((Some(cargo_name.to_owned()), path))),
None => Some(Err("crate has not yet been built".to_owned())),
Expand Down
2 changes: 1 addition & 1 deletion crates/rust-analyzer/src/cargo_target_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ impl CargoTargetSpec {
buf.push("--example".to_owned());
buf.push(self.target);
}
TargetKind::Lib => {
TargetKind::Lib { is_proc_macro: _ } => {
buf.push("--lib".to_owned());
}
TargetKind::Other | TargetKind::BuildScript => (),
Expand Down
2 changes: 1 addition & 1 deletion crates/rust-analyzer/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ config_data! {
cargo_buildScripts_overrideCommand: Option<Vec<String>> = "null",
/// Rerun proc-macros building/build-scripts running when proc-macro
/// or build-script sources change and are saved.
cargo_buildScripts_rebuildOnSave: bool = "false",
cargo_buildScripts_rebuildOnSave: bool = "true",
/// Use `RUSTC_WRAPPER=rust-analyzer` when running build scripts to
/// avoid checking unnecessary things.
cargo_buildScripts_useRustcWrapper: bool = "true",
Expand Down
39 changes: 28 additions & 11 deletions crates/rust-analyzer/src/global_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ use crossbeam_channel::{unbounded, Receiver, Sender};
use flycheck::FlycheckHandle;
use hir::Change;
use ide::{Analysis, AnalysisHost, Cancellable, FileId};
use ide_db::base_db::{CrateId, FileLoader, ProcMacroPaths, SourceDatabase};
use ide_db::base_db::{
salsa::ParallelDatabase, CrateId, FileLoader, ProcMacroPaths, SourceDatabase,
};
use load_cargo::SourceRootConfig;
use lsp_types::{SemanticTokens, Url};
use nohash_hasher::IntMap;
Expand Down Expand Up @@ -74,9 +76,11 @@ pub(crate) struct GlobalState {
pub(crate) last_reported_status: Option<lsp_ext::ServerStatusParams>,

// proc macros
pub(crate) proc_macro_changed: bool,
pub(crate) proc_macro_clients: Arc<[anyhow::Result<ProcMacroServer>]>,

// build scripts
pub(crate) build_script_changed: bool,

// Flycheck
pub(crate) flycheck: Arc<[FlycheckHandle]>,
pub(crate) flycheck_sender: Sender<flycheck::Message>,
Expand Down Expand Up @@ -187,9 +191,10 @@ impl GlobalState {
source_root_config: SourceRootConfig::default(),
config_errors: Default::default(),

proc_macro_changed: false,
proc_macro_clients: Arc::from_iter([]),

build_script_changed: false,

flycheck: Arc::from_iter([]),
flycheck_sender,
flycheck_receiver,
Expand Down Expand Up @@ -287,6 +292,9 @@ impl GlobalState {
if reload::should_refresh_for_change(&path, file.change_kind) {
workspace_structure_change = Some((path.clone(), false));
}
if AsRef::<std::path::Path>::as_ref(&path).ends_with("build.rs") {
self.build_script_changed = true;
}
if file.is_created_or_deleted() {
has_structure_changes = true;
workspace_structure_change =
Expand Down Expand Up @@ -333,7 +341,6 @@ impl GlobalState {
self.analysis_host.apply_change(change);

{
let raw_database = self.analysis_host.raw_database();
// FIXME: ideally we should only trigger a workspace fetch for non-library changes
// but something's going wrong with the source root business when we add a new local
// crate see https://github.com/rust-lang/rust-analyzer/issues/13029
Expand All @@ -343,13 +350,23 @@ impl GlobalState {
force_crate_graph_reload,
);
}
self.proc_macro_changed =
changed_files.iter().filter(|file| !file.is_created_or_deleted()).any(|file| {
let crates = raw_database.relevant_crates(file.file_id);
let crate_graph = raw_database.crate_graph();

crates.iter().any(|&krate| crate_graph[krate].is_proc_macro)
});
self.task_pool.handle.spawn(stdx::thread::ThreadIntent::Worker, {
let db = self.analysis_host.raw_database().snapshot();
move || {
Task::BuildScriptsMayHaveChanged(
changed_files.iter().filter(|file| !file.is_created_or_deleted()).any(
|file| {
// FIXME: We should check for build script related changes as well
// but thats a lot more tricky to pull off.
let crates = db.relevant_crates(file.file_id);
let crate_graph = db.crate_graph();

crates.iter().any(|&krate| crate_graph[krate].is_proc_macro)
},
),
)
}
});
}

true
Expand Down
12 changes: 5 additions & 7 deletions crates/rust-analyzer/src/handlers/notification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,9 @@ pub(crate) fn handle_did_save_text_document(
state: &mut GlobalState,
params: DidSaveTextDocumentParams,
) -> anyhow::Result<()> {
if state.config.script_rebuild_on_save() && state.proc_macro_changed {
// reset the flag
state.proc_macro_changed = false;
// rebuild the proc macros
state.fetch_build_data_queue.request_op("ScriptRebuildOnSave".to_owned(), ());
if state.config.script_rebuild_on_save() && state.build_script_changed {
state.build_script_changed = false;
state.fetch_build_data_queue.request_op("buildScriptsRebuildOnSave".to_owned(), ());
}

if let Ok(vfs_path) = from_proto::vfs_path(&params.text_document.uri) {
Expand All @@ -143,7 +141,7 @@ pub(crate) fn handle_did_save_text_document(
if reload::should_refresh_for_change(abs_path, ChangeKind::Modify) {
state
.fetch_workspaces_queue
.request_op(format!("DidSaveTextDocument {abs_path}"), false);
.request_op(format!("workspace vfs file change saved {abs_path}"), false);
}
}

Expand Down Expand Up @@ -221,7 +219,7 @@ pub(crate) fn handle_did_change_workspace_folders(

if !config.has_linked_projects() && config.detached_files().is_empty() {
config.rediscover_workspaces();
state.fetch_workspaces_queue.request_op("client workspaces changed".to_string(), false)
state.fetch_workspaces_queue.request_op("workspaces folders changed".to_string(), false)
}

Ok(())
Expand Down
4 changes: 2 additions & 2 deletions crates/rust-analyzer/src/handlers/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,15 @@ use crate::{

pub(crate) fn handle_workspace_reload(state: &mut GlobalState, _: ()) -> anyhow::Result<()> {
state.proc_macro_clients = Arc::from_iter([]);
state.proc_macro_changed = false;
state.build_script_changed = false;

state.fetch_workspaces_queue.request_op("reload workspace request".to_string(), false);
Ok(())
}

pub(crate) fn handle_proc_macros_rebuild(state: &mut GlobalState, _: ()) -> anyhow::Result<()> {
state.proc_macro_clients = Arc::from_iter([]);
state.proc_macro_changed = false;
state.build_script_changed = false;

state.fetch_build_data_queue.request_op("rebuild proc macros request".to_string(), ());
Ok(())
Expand Down
Loading

0 comments on commit 443fc29

Please sign in to comment.