Re: [PATCH v4 1/2] scripts: generate_rust_analyzer.py: add versioning infrastructure

[Tamir Duberstein]

On Sun, Apr 26, 2026 at 8:42 AM Jesung Yang <y.j3ms.n@xxxxxxxxx> wrote:
>
> On Sat, Apr 25, 2026 at 3:12 PM Tamir Duberstein <tamird@xxxxxxxxxx> wrote:
> > On 2026-04-25 12:15:03+00:00, Jesung Yang wrote:
> > > On Thu, Mar 19, 2026 at 3:02 PM Tamir Duberstein <tamird@xxxxxxxxxx> wrote:
> > > >
> > > > It would be great to avoid having to sprinkle this everywhere, though I admit
> > > > it's not immediately obvious to me how to achieve it. Maybe we can live with it
> > > > until the MSRV bump. Again, would be great to structure this in a way that
> > > > makes that cleanup simple when the time comes.
> > >
> > > I considered using function decorators, but on second thought, that
> > > would just be another way of sprinkling functions around.
> >
> > Can you explain how function decorators would help? Not entirely clear
> > to me.
>
> The idea was to augment all `append_*` functions with a decorator that
> contains the `sysroot_deps` logic so that we don't have to call
> `sysroot_deps` multiple times at the call site. Something like the
> following (not tested, just for demonstration):
>
> -------------------------------------------------
> def filter_deps(f):
>     def wrapper(*args, **kwargs):
>         args = list(args)
>         args[2] = [dep for dep in args[2] if dep in not None]
>         return f(*args, **kwargs)
>     return wrapper
>
> @filter_deps
> def append_proc_macro_crate(
>     display_name: str,
>     root_module: pathlib.Path,
>     deps: List[Dependency],
>     *,
>     ...,
> ) -> Dependency:
>     # ...
>
> @filter_deps
> def append_crate(
>     display_name: str,
>     root_module: pathlib.Path,
>     deps: List[Dependency],
>     *,
>     ...,
> ) -> Dependency:
>     # ...
> -------------------------------------------------
>
> Since this only touches the function definitions, we have fewer
> things to clean up.
>
> Note that this isn't a clean solution: we'd need to keep mypy happy,
> `deps` isn't always the third parameter, and there's a discrepancy
> between the written signature and the wrapped behavior (the latter
> effectively changes the type of `deps` from `List[Dependency]` to
> `List[Optional[Dependency]]`).

Right, and also the way you've written the decorator won't appease
mypy; you'll need type annotations. If the goal is to change the type
of `deps` to a list of optionals, a decorator is a nice way to do that
without polluting the git history.

I believe the proper way to annotate this kind of decorator is using
ParamSpec (https://docs.python.org/3/library/typing.html#typing.ParamSpec)
which might not be supported in Python 3.9. That might be OK though -
our minimum Python for type checking need not equal our minimum Python
at runtime.

>
> > > In my view, the cleanup should still be fairly straightforward even with
> > > the current `sysroot_deps` approach. We could just remove the function
> > > and let mypy (or an LSP) flag the undefined function error/warnings.
> > > Does that sound reasonable to you?
> >
> > Mechanically that would work of course, but it would result in a
> > flip-flop in the git history. I was hoping to avoid changes at the
> > caller for this reason.
>
> That makes sense. I don't have a clean solution to avoid this right now;
> I'll keep mulling it over in the background while we finalize the other
> points.
>
> > > > There should be a note here explaining that this should be bumped when MSRV is
> > > > bumped, and how to obtain the new values that should go here. It's really
> > > > important that the string MSRV appears as it's a likely grep target for when
> > > > that pointer is updated.
> > >
> > > I'll add a comment about the MSRV bump. Unfortunately, there isn't a
> > > reliable way to programmatically retrieve the matching rust-analyzer
> > > release AFAICT. I typically follow these steps:
> > >
> > > 1. Look for the commit history of `src/tools/rust-analyzer` in the
> > >    upstream Rust repository (either locally or via GitHub [1]).
> > > 2. Check the latest merge commit from on the rust-analyzer side, which
> > >    usually starts with "Merge pull request #<PR_ID>".
> > > 3. Search for that <PR_ID> on the rust-analyzer release page [2].
> > > 4. Grab the version string and release date.
> > >
> > > Perhaps the better approach is to perform these manual checks upfront
> > > for versions 1.86 through 1.94, at which point this version
> > > infrastructure can finally be removed.
> > >
> > > [1] https://github.com/rust-lang/rust/commits/<TAG>/src/tools/rust-analyzer
> > > [2] https://github.com/rust-lang/rust-analyzer/releases
> >
> > I defer to you on whether or not to do this up front, but I'd like us to
> > document the procedure either way. If possible, prefer to describe it in
> > terms of the git CLI rather than GitHub.
>
> As a best effort, we can do something like this:
>
> ```bash
> git clone https://github.com/rust-lang/rust
> git clone https://github.com/rust-lang/rust-analyzer
>
> RUST_VERSION=1.85.0
> LOOKAHEAD=100
>
> subject_string=$(
>   git -C ./rust log -n $LOOKAHEAD --format='%s' $RUST_VERSION \
>     -- src/tools/rust-analyzer | grep 'Merge pull request #'
> )
> readarray -t subject_array <<< "$subject_string"
>
> git -C ./rust-analyzer log --oneline --fixed-strings
> "${subject_array[@]/#/--grep=}"
> ```
>
> This provides merge commit candidates that can be searched on the
> rust-analyzer GitHub release page. Since rust-analyzer version strings
> are only available on GitHub, a plain local git CLI cannot retrieve them
> directly. The GitHub CLI (`gh`) could likely automate the final step,
> but I'm unsure if suggesting `gh` as a requirement is the right choice
> here.

The rust-analyzer releases are all git tags; once you figure out the
right tag, you can navigate to
https://github.com/rust-lang/rust-analyzer/releases/tag/<tag> to get
the version number.

>
> Best regards,
> Jesung
>