Skip to content
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

fix: Fix build scripts not being rebuilt in some occasions #16247

Merged
merged 1 commit into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading