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 Feb 14, 2024
1 parent a02a219 commit fa4af62
Show file tree
Hide file tree
Showing 17 changed files with 122 additions and 78 deletions.
8 changes: 4 additions & 4 deletions crates/hir-def/src/nameres/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,9 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, def_map: DefMap, tree_id: TreeI
if it.disabled {
CustomProcMacroExpander::disabled()
} else {
CustomProcMacroExpander::new(hir_expand::proc_macro::ProcMacroId(
idx as u32,
))
CustomProcMacroExpander::new(
hir_expand::proc_macro::ProcMacroId::new(idx as u32),
)
},
)
})
Expand Down Expand Up @@ -2354,7 +2354,7 @@ impl ModCollector<'_, '_> {
resolved_res.resolved_def.take_macros().map(|it| db.macro_def(it))
},
) {
// FIXME: if there were errors, this mightve been in the eager expansion from an
// FIXME: if there were errors, this might've been in the eager expansion from an
// unresolved macro, so we need to push this into late macro resolution. see fixme above
if res.err.is_none() {
// Legacy macros need to be expanded immediately, so that any macros they produce
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 @@ -227,8 +227,8 @@ pub enum MacroCallKind {
},
Attr {
ast_id: AstId<ast::Item>,
// FIXME: This is being interned, subtrees can vary 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
2 changes: 1 addition & 1 deletion crates/ide/src/parent_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ pub(crate) fn parent_module(db: &RootDatabase, position: FilePosition) -> Vec<Na
}
}

/// Returns `Vec` for the same reason as `parent_module`
/// This returns `Vec` because a module may be included from several places.
pub(crate) fn crates_for(db: &RootDatabase, file_id: FileId) -> Vec<CrateId> {
db.relevant_crates(file_id)
.iter()
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 @@ -24,7 +24,7 @@ use toolchain::Tool;

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

#[derive(Debug, Default, Clone, PartialEq, Eq)]
Expand Down Expand Up @@ -467,7 +467,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 @@ -189,8 +189,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 @@ -199,7 +197,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 @@ -216,8 +217,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 @@ -370,7 +371,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
36 changes: 18 additions & 18 deletions crates/project-model/src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,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 @@ -985,15 +985,15 @@ 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];

let Some(file_id) = load(root) else { continue };

Expand All @@ -1005,19 +1005,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 @@ -1215,9 +1220,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 @@ -1227,7 +1232,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 @@ -1236,12 +1241,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 @@ -1303,7 +1303,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 @@ -1353,7 +1353,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 @@ -1364,7 +1364,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 @@ -112,7 +112,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
41 changes: 27 additions & 14 deletions crates/rust-analyzer/src/global_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ 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::{CrateId, ProcMacroPaths};
use load_cargo::SourceRootConfig;
use lsp_types::{SemanticTokens, Url};
use nohash_hasher::IntMap;
Expand Down Expand Up @@ -74,8 +74,8 @@ 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>]>,
pub(crate) build_deps_changed: bool,

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

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

build_deps_changed: false,

flycheck: Arc::from_iter([]),
flycheck_sender,
flycheck_receiver,
Expand Down Expand Up @@ -300,12 +301,19 @@ impl GlobalState {
if let Some(path) = vfs_path.as_path() {
let path = path.to_path_buf();
if reload::should_refresh_for_change(&path, file.kind()) {
workspace_structure_change = Some((path.clone(), false));
workspace_structure_change = Some((
path.clone(),
false,
AsRef::<std::path::Path>::as_ref(&path).ends_with("build.rs"),
));
}
if file.is_created_or_deleted() {
has_structure_changes = true;
workspace_structure_change =
Some((path, self.crate_graph_file_dependencies.contains(vfs_path)));
workspace_structure_change = Some((
path,
self.crate_graph_file_dependencies.contains(vfs_path),
false,
));
} else if path.extension() == Some("rs".as_ref()) {
modified_rust_files.push(file.file_id);
}
Expand Down Expand Up @@ -346,23 +354,28 @@ impl GlobalState {
};

self.analysis_host.apply_change(change);

{
let raw_database = self.analysis_host.raw_database();
if !matches!(&workspace_structure_change, Some((.., true))) {
_ = self
.deferred_task_queue
.sender
.send(crate::main_loop::QueuedTask::CheckProcMacroSources(modified_rust_files));
}
// 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
if let Some((path, force_crate_graph_reload)) = workspace_structure_change {
if let Some((path, force_crate_graph_reload, build_scripts_touched)) =
workspace_structure_change
{
self.fetch_workspaces_queue.request_op(
format!("workspace vfs file change: {path}"),
force_crate_graph_reload,
);
if build_scripts_touched {
self.fetch_build_data_queue.request_op(format!("build.rs changed: {path}"), ());
}
}
self.proc_macro_changed = modified_rust_files.into_iter().any(|file_id| {
let crates = raw_database.relevant_crates(file_id);
let crate_graph = raw_database.crate_graph();

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

true
Expand Down
12 changes: 6 additions & 6 deletions crates/rust-analyzer/src/handlers/notification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,11 @@ 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_deps_changed {
state.build_deps_changed = false;
state
.fetch_build_data_queue
.request_op("build_deps_changed - save notification".to_owned(), ());
}

if let Ok(vfs_path) = from_proto::vfs_path(&params.text_document.uri) {
Expand All @@ -158,7 +158,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
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_deps_changed = false;

state.fetch_workspaces_queue.request_op("reload workspace request".to_owned(), 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_deps_changed = false;

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

0 comments on commit fa4af62

Please sign in to comment.