fix: Fix build scripts not being rebuilt in some occasions

This commit is contained in:
Lukas Wirth 2024-01-04 10:19:25 +01:00
parent a02a219773
commit 1e6cef94df
16 changed files with 121 additions and 77 deletions

View file

@ -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),
)
},
)
})
@ -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

View file

@ -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]`.
///

View file

@ -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 {

View file

@ -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()

View file

@ -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)]
@ -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)

View file

@ -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>,
}
@ -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,
@ -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,
};
}
@ -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);

View file

@ -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));
@ -985,7 +985,7 @@ 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.
@ -993,7 +993,7 @@ fn cargo_to_crate_graph(
// 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 };
@ -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));
@ -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,
@ -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(),
@ -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);
}
@ -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>,
@ -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 {
@ -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())),

View file

@ -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 => (),

View file

@ -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",

View file

@ -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;
@ -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]>,
@ -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,
@ -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);
}
@ -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

View file

@ -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) {
@ -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);
}
}

View file

@ -52,7 +52,7 @@ 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(())
@ -60,7 +60,7 @@ pub(crate) fn handle_workspace_reload(state: &mut GlobalState, _: ()) -> anyhow:
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(())

View file

@ -8,11 +8,10 @@ use std::{
use always_assert::always;
use crossbeam_channel::{select, Receiver};
use ide_db::base_db::{SourceDatabaseExt, VfsPath};
use ide_db::base_db::{SourceDatabase, SourceDatabaseExt, VfsPath};
use lsp_server::{Connection, Notification, Request};
use lsp_types::notification::Notification as _;
use stdx::thread::ThreadIntent;
use triomphe::Arc;
use vfs::FileId;
use crate::{
@ -76,6 +75,7 @@ impl fmt::Display for Event {
#[derive(Debug)]
pub(crate) enum QueuedTask {
CheckIfIndexed(lsp_types::Url),
CheckProcMacroSources(Vec<FileId>),
}
#[derive(Debug)]
@ -88,6 +88,7 @@ pub(crate) enum Task {
FetchWorkspace(ProjectWorkspaceProgress),
FetchBuildData(BuildDataProgress),
LoadProcMacros(ProcMacroProgress),
BuildDepsHaveChanged,
}
#[derive(Debug)]
@ -357,9 +358,7 @@ impl GlobalState {
}
// Refresh inlay hints if the client supports it.
if (self.send_hint_refresh_query || self.proc_macro_changed)
&& self.config.inlay_hints_refresh()
{
if self.send_hint_refresh_query && self.config.inlay_hints_refresh() {
self.send_request::<lsp_types::request::InlayHintRefreshRequest>((), |_, _| ());
self.send_hint_refresh_query = false;
}
@ -554,16 +553,7 @@ impl GlobalState {
if let Err(e) = self.fetch_workspace_error() {
tracing::error!("FetchWorkspaceError:\n{e}");
}
let old = Arc::clone(&self.workspaces);
self.switch_workspaces("fetched workspace".to_owned());
let workspaces_updated = !Arc::ptr_eq(&old, &self.workspaces);
if self.config.run_build_scripts() && workspaces_updated {
self.fetch_build_data_queue
.request_op("workspace updated".to_owned(), ());
}
(Progress::End, None)
}
};
@ -607,6 +597,7 @@ impl GlobalState {
self.report_progress("Loading", state, msg, None, None);
}
}
Task::BuildDepsHaveChanged => self.build_deps_changed = true,
}
}
@ -685,6 +676,25 @@ impl GlobalState {
}
});
}
QueuedTask::CheckProcMacroSources(modified_rust_files) => {
let crate_graph = self.analysis_host.raw_database().crate_graph();
let snap = self.snapshot();
self.task_pool.handle.spawn_with_sender(stdx::thread::ThreadIntent::Worker, {
move |sender| {
if modified_rust_files.into_iter().any(|file_id| {
// FIXME: Check whether these files could be build script related
match snap.analysis.crates_for(file_id) {
Ok(crates) => {
crates.iter().any(|&krate| crate_graph[krate].is_proc_macro)
}
_ => false,
}
}) {
sender.send(Task::BuildDepsHaveChanged).unwrap();
}
}
});
}
}
}

View file

@ -83,7 +83,7 @@ impl GlobalState {
}
if self.config.linked_or_discovered_projects() != old_config.linked_or_discovered_projects()
{
self.fetch_workspaces_queue.request_op("linked projects changed".to_owned(), false)
self.fetch_workspaces_queue.request_op("discovered projects changed".to_owned(), false)
} else if self.config.flycheck() != old_config.flycheck() {
self.reload_flycheck();
}
@ -106,9 +106,11 @@ impl GlobalState {
};
let mut message = String::new();
if self.proc_macro_changed {
if self.build_deps_changed {
status.health = lsp_ext::Health::Warning;
message.push_str("Proc-macros have changed and need to be rebuilt.\n\n");
message.push_str(
"Proc-macros and/or build scripts have changed and need to be rebuilt.\n\n",
);
}
if self.fetch_build_data_error().is_err() {
status.health = lsp_ext::Health::Warning;
@ -408,6 +410,10 @@ impl GlobalState {
if *force_reload_crate_graph {
self.recreate_crate_graph(cause);
}
if self.build_deps_changed && self.config.run_build_scripts() {
self.build_deps_changed = false;
self.fetch_build_data_queue.request_op("build_deps_changed".to_owned(), ());
}
// Current build scripts do not match the version of the active
// workspace, so there's nothing for us to update.
return;
@ -419,6 +425,11 @@ impl GlobalState {
// we don't care about build-script results, they are stale.
// FIXME: can we abort the build scripts here?
self.workspaces = Arc::new(workspaces);
if self.config.run_build_scripts() {
self.build_deps_changed = false;
self.fetch_build_data_queue.request_op("workspace updated".to_owned(), ());
}
}
if let FilesWatcher::Client = self.config.files().watcher {

View file

@ -71,7 +71,7 @@ cargo check --quiet --workspace --message-format=json --all-targets
```
.
--
[[rust-analyzer.cargo.buildScripts.rebuildOnSave]]rust-analyzer.cargo.buildScripts.rebuildOnSave (default: `false`)::
[[rust-analyzer.cargo.buildScripts.rebuildOnSave]]rust-analyzer.cargo.buildScripts.rebuildOnSave (default: `true`)::
+
--
Rerun proc-macros building/build-scripts running when proc-macro

View file

@ -590,7 +590,7 @@
},
"rust-analyzer.cargo.buildScripts.rebuildOnSave": {
"markdownDescription": "Rerun proc-macros building/build-scripts running when proc-macro\nor build-script sources change and are saved.",
"default": false,
"default": true,
"type": "boolean"
},
"rust-analyzer.cargo.buildScripts.useRustcWrapper": {