Skip to content

Fix bug where rustdoc-js tester would not pick the right search.js file if there is more than one #145359

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

Merged
merged 2 commits into from
Aug 20, 2025

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Aug 13, 2025

It happened to me quite a few times recently when I worked on the search index:

  1. I make a change in search.js
  2. I run rustdoc-js tests
  3. nothing changes

So my solution was to simply remove the folder, but it's really suboptimal. With this PR, it now picks the most recently modified file.

cc @lolbinarycat

@rustbot
Copy link
Collaborator

rustbot commented Aug 13, 2025

r? @notriddle

rustbot has assigned @notriddle.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Aug 13, 2025
Copy link
Contributor

@lolbinarycat lolbinarycat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, besides two tiny edge cases that probably don't matter:

  1. if another search-related js file is added, the regex may be too permissive (can just be fixed whenever it comes up, and hopefully won't be too much of a pain to debug)
  2. if somehow no file matches, this will give an index out of bounds error (but it should also give a stack trace pointing right to the code that needs updating)

Also considered if this could hit editor temporary files, but since the regex checks prefix and suffix, it never should.

@@ -452,7 +467,7 @@ function loadSearchJS(doc_folder, resource_suffix) {
};

const staticFiles = path.join(doc_folder, "static.files");
const searchJs = fs.readdirSync(staticFiles).find(f => f.match(/search.*\.js$/));
const searchJs = mostRecentMatch(staticFiles, /search.*\.js$/);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const searchJs = mostRecentMatch(staticFiles, /search.*\.js$/);
const searchJs = mostRecentMatch(staticFiles, /search-[0-9a-f]{8}.*\.js$/);

What do you think about making the regex a bit stricter in case we add another search-related js module at some point? The .* is still needed because of --resource-suffix, but maybe it would be a bit more reliable?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, applied it.

@GuillaumeGomez GuillaumeGomez force-pushed the correctly-pick-search.js branch from 3c76b67 to 97400fd Compare August 16, 2025 21:59
@rustbot
Copy link
Collaborator

rustbot commented Aug 16, 2025

This PR was rebased onto a different master commit! Check out the changes with our range-diff.

@GuillaumeGomez
Copy link
Member Author

I added a check for the number of matching files and it emits an exception if it's 0.

@bors delegate=lolbinarycat

@bors
Copy link
Collaborator

bors commented Aug 16, 2025

✌️ @lolbinarycat, you can now approve this pull request!

If @GuillaumeGomez told you to "r=me" after making some further change, please make that change, then do @bors r=@GuillaumeGomez

@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez GuillaumeGomez force-pushed the correctly-pick-search.js branch from 97400fd to 826ebde Compare August 16, 2025 22:21
@lolbinarycat
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 17, 2025

📌 Commit 826ebde has been approved by lolbinarycat

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 17, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Aug 17, 2025
…ch.js, r=lolbinarycat

Fix bug where `rustdoc-js` tester would not pick the right `search.js` file if there is more than one

It happened to me quite a few times recently when I worked on the search index:
1. I make a change in search.js
2. I run `rustdoc-js` tests
3. nothing changes

So my solution was to simply remove the folder, but it's really suboptimal. With this PR, it now picks the most recently modified file.

cc ``@lolbinarycat``
jhpratt added a commit to jhpratt/rust that referenced this pull request Aug 17, 2025
…ch.js, r=lolbinarycat

Fix bug where `rustdoc-js` tester would not pick the right `search.js` file if there is more than one

It happened to me quite a few times recently when I worked on the search index:
1. I make a change in search.js
2. I run `rustdoc-js` tests
3. nothing changes

So my solution was to simply remove the folder, but it's really suboptimal. With this PR, it now picks the most recently modified file.

cc ```@lolbinarycat```
Zalathar added a commit to Zalathar/rust that referenced this pull request Aug 18, 2025
…ch.js, r=lolbinarycat

Fix bug where `rustdoc-js` tester would not pick the right `search.js` file if there is more than one

It happened to me quite a few times recently when I worked on the search index:
1. I make a change in search.js
2. I run `rustdoc-js` tests
3. nothing changes

So my solution was to simply remove the folder, but it's really suboptimal. With this PR, it now picks the most recently modified file.

cc ````@lolbinarycat````
bors added a commit that referenced this pull request Aug 18, 2025
Rollup of 19 pull requests

Successful merges:

 - #140956 (`impl PartialEq<{str,String}> for {Path,PathBuf}`)
 - #141744 (Stabilize `ip_from`)
 - #144804 (Don't warn on never to any `as` casts as unreachable)
 - #144983 (Rehome 37 `tests/ui/issues/` tests to other subdirectories under `tests/ui/`)
 - #145025 (run spellcheck as a tidy extra check in ci)
 - #145041 (rework GAT borrowck limitation error)
 - #145243 (take attr style into account in diagnostics)
 - #145359 (Fix bug where `rustdoc-js` tester would not pick the right `search.js` file if there is more than one)
 - #145429 (Couple of codegen_fn_attrs improvements)
 - #145452 (Do not strip binaries in bootstrap everytime if they are unchanged)
 - #145486 (Fix `unicode_data.rs` mention message)
 - #145489 (library: Migrate from `cfg_if` to `cfg_select`)
 - #145493 (remove `should_render` in `PrintAttribute` derive)
 - #145500 (Port must_use to the new target checking)
 - #145505 (Simplify span caches)
 - #145511 (Rust build fails on OpenBSD after using file_lock feature)
 - #145516 (Weekly `cargo update`)
 - #145533 (Reorder `lto` options from most to least optimizing)
 - #145550 (Avoid using `()` in `derive(From)` output.)

r? `@ghost`
`@rustbot` modify labels: rollup
Zalathar added a commit to Zalathar/rust that referenced this pull request Aug 18, 2025
…ch.js, r=lolbinarycat

Fix bug where `rustdoc-js` tester would not pick the right `search.js` file if there is more than one

It happened to me quite a few times recently when I worked on the search index:
1. I make a change in search.js
2. I run `rustdoc-js` tests
3. nothing changes

So my solution was to simply remove the folder, but it's really suboptimal. With this PR, it now picks the most recently modified file.

cc `````@lolbinarycat`````
bors added a commit that referenced this pull request Aug 19, 2025
Rollup of 19 pull requests

Successful merges:

 - #140956 (`impl PartialEq<{str,String}> for {Path,PathBuf}`)
 - #141744 (Stabilize `ip_from`)
 - #142681 (Remove the `#[no_sanitize]` attribute in favor of `#[sanitize(xyz = "on|off")]`)
 - #142871 (Trivial improve doc for transpose )
 - #144252 (Do not copy .rmeta files into the sysroot of the build compiler during check of rustc/std)
 - #144476 (rustdoc-search: search backend with partitioned suffix tree)
 - #144567 (Fix RISC-V Test Failures in ./x test for Multiple Codegen Cases)
 - #144804 (Don't warn on never to any `as` casts as unreachable)
 - #144960 ([RTE-513] Ignore sleep_until test on SGX)
 - #145013 (overhaul `&mut` suggestions in borrowck errors)
 - #145041 (rework GAT borrowck limitation error)
 - #145243 (take attr style into account in diagnostics)
 - #145405 (cleanup: use run_in_tmpdir in run-make/rustdoc-scrape-examples-paths)
 - #145432 (cg_llvm: Small cleanups to `owned_target_machine`)
 - #145484 (Remove `LlvmArchiveBuilder` and supporting code/bindings)
 - #145557 (Fix uplifting in `Assemble` step)
 - #145563 (Remove the `From` derive macro from prelude)
 - #145565 (Improve context of bootstrap errors in CI)
 - #145584 (interpret: avoid forcing all integer newtypes into memory during clear_provenance)

Failed merges:

 - #145359 (Fix bug where `rustdoc-js` tester would not pick the right `search.js` file if there is more than one)
 - #145573 (Add an experimental unsafe(force_target_feature) attribute.)

r? `@ghost`
`@rustbot` modify labels: rollup
@jieyouxu
Copy link
Member

Merge conflict.
@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 19, 2025
@GuillaumeGomez GuillaumeGomez force-pushed the correctly-pick-search.js branch from 826ebde to 2ebe679 Compare August 19, 2025 13:16
@rustbot
Copy link
Collaborator

rustbot commented Aug 19, 2025

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@GuillaumeGomez
Copy link
Member Author

@bors r=lolbinarycat rollup

@bors
Copy link
Collaborator

bors commented Aug 19, 2025

📌 Commit 2ebe679 has been approved by lolbinarycat

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 19, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Aug 19, 2025
…ch.js, r=lolbinarycat

Fix bug where `rustdoc-js` tester would not pick the right `search.js` file if there is more than one

It happened to me quite a few times recently when I worked on the search index:
1. I make a change in search.js
2. I run `rustdoc-js` tests
3. nothing changes

So my solution was to simply remove the folder, but it's really suboptimal. With this PR, it now picks the most recently modified file.

cc `@lolbinarycat`
jhpratt added a commit to jhpratt/rust that referenced this pull request Aug 19, 2025
…ch.js, r=lolbinarycat

Fix bug where `rustdoc-js` tester would not pick the right `search.js` file if there is more than one

It happened to me quite a few times recently when I worked on the search index:
1. I make a change in search.js
2. I run `rustdoc-js` tests
3. nothing changes

So my solution was to simply remove the folder, but it's really suboptimal. With this PR, it now picks the most recently modified file.

cc ``@lolbinarycat``
bors added a commit that referenced this pull request Aug 20, 2025
Rollup of 13 pull requests

Successful merges:

 - #139357 (Fix parameter order for `_by()` variants of `min` / `max`/ `minmax` in `std::cmp`)
 - #140314 (Rustdoc: typecheck scrape-examples.js)
 - #140794 (mention lint group in default level lint note)
 - #145006 (Clarify EOF handling for `BufRead::skip_until`)
 - #145252 (Demote x86_64-apple-darwin to Tier 2 with host tools)
 - #145359 (Fix bug where `rustdoc-js` tester would not pick the right `search.js` file if there is more than one)
 - #145381 (Implement feature `int_lowest_highest_one` for integer and NonZero types)
 - #145417 (std_detect: RISC-V platform guide documentation)
 - #145531 (Add runtime detection for APX-F and AVX10)
 - #145619 (`std_detect`: Use `rustc-std-workspace-*` to pull in `compiler-builtins`)
 - #145622 (Remove the std workspace patch for `compiler-builtins`)
 - #145623 (Pretty print the name of an future from calling async closure)
 - #145626 (add a fallback implementation for the `prefetch_*` intrinsics )

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a8feea8 into rust-lang:master Aug 20, 2025
10 checks passed
@rustbot rustbot added this to the 1.91.0 milestone Aug 20, 2025
rust-timer added a commit that referenced this pull request Aug 20, 2025
Rollup merge of #145359 - GuillaumeGomez:correctly-pick-search.js, r=lolbinarycat

Fix bug where `rustdoc-js` tester would not pick the right `search.js` file if there is more than one

It happened to me quite a few times recently when I worked on the search index:
1. I make a change in search.js
2. I run `rustdoc-js` tests
3. nothing changes

So my solution was to simply remove the folder, but it's really suboptimal. With this PR, it now picks the most recently modified file.

cc ```@lolbinarycat```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants