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

A tool to perform renames #151

Open
roberth opened this issue Nov 2, 2022 · 5 comments
Open

A tool to perform renames #151

roberth opened this issue Nov 2, 2022 · 5 comments
Labels
enhancement New feature or request

Comments

@roberth
Copy link

roberth commented Nov 2, 2022

Description

When renaming a file in a collection of Nix files, the relative path expressions that referenced the old location become invalid.
The Nixpkgs architecture team is working on a cleanup operation that will move a huge number of files. Being able to do so in an automated and accurate manner would be a game changer.

The code that supports this operation may also benefit rnix-lsp, although the LSP rename functionality doesn't appear to be as advanced as, say, IntelliJ's.

Considered alternatives

We might hack the paths using sed and friends, but this runs the risk of breaking some paths whose use is optional and therefore not revealed by testing the evaluation.

Additional context

This is a call for help.
I don't have the rust experience to pull this off, but I know rnix is well suited, because it can preserve syntax and it supports an lsp.

Considering the O(n²) nature of all possible file-to-file references, we'll want to perform the renames in batches. Non-Nix files may be referenced and renamed.

Example interface:

cd nixpkgs
rnix-mv --project . \
  --from pkgs/os-specific/linux/mdadm --to pkg/mdadm \
  --from pkgs/desktops/xfce/core/xfwm4 --to pkg/xfwm4

--project scans the passed directory recursively for .nix files that may reference the moved files.

Our use case is https://github.com/nixpkgs-architecture/auto-called-packages
If you're interested on working on this, reach out to us on https://matrix.to/#/#nixpkgs-architecture:nixos.org

@roberth roberth added the enhancement New feature or request label Nov 2, 2022
@infinisil
Copy link

I thought about how to implement this a bit already:

  • This should be split into two binaries
    • nix-file-index, creating an index from all paths to the Nix files that reference them
      • For simplicity, the index can be an sqlite database. The schema should just be a single table containing reference from Nix files to arbitrary paths. sqlite can then do the reverse index for us automatically.
    • nix-file-rename which takes the index from nix-file-index and uses it to perform the rename quickly
      • This would be just like a mv command, taking two arguments, source and target
  • Interestingly, such a tool doesn't need to be Nix-specific. If done right, nix-file-rename can be a more general file-rename
  • It might be good to have a initial path normalization step/tool, which makes sure nix paths are as direct as they can be (so e.g. ../../foo/bar in foo/baz/default.nix gets turned into ../bar. By doing this first, the rename tool doesn't need to care about preserving the old path and can always recompute the relative path itself.
    • Nix paths turn /foo/bar/../baz into /foo/baz without checking for what bar is (could be a symlink). This is however rare enough that it isn't really worth looking into
  • Such a rename tool can notably still cause evaluation changes, though this should be rare:
    • One reason is simply that the code can be imported into the store, and if the files now live in a different place, you get different results.
    • Another one is that paths can be toString'd, which of course changes when the path updates.
  • Renames should be reversible, nix-file-rename a b && nix-file-rename b a is a no-op
  • Note that not only need file references to be tracked in the index but also folders, since you can reference just folders (and move entire folders)

@roberth
Copy link
Author

roberth commented Nov 2, 2022

@infinisil I was trying to describe the minimal implementation that would be viable for the un-categorization refactoring. Your suggestions seem to ask for more than what is strictly necessary.

I thought about how to implement this a bit already:

  • This should be split into two binaries

    • nix-file-index, creating an index from all paths to the Nix files that reference them

I disagree. This is an optimization that's only necessary when the performance isn't good enough.
Keeping the index in memory is simpler.

* This would be just like a `mv` command, taking two arguments, source and target

I agree that this would be a familiar interface, but the performance optimization required for this is a lot to ask.

  • Interestingly, such a tool doesn't need to be Nix-specific. If done right, nix-file-rename can be a more general file-rename

This is LSP or other standardization territory. It's not necessary.

  • It might be good to have a initial path normalization step/tool, which makes sure nix paths are as direct as they can be (so e.g. ../../foo/bar in foo/baz/default.nix gets turned into ../bar. By doing this first, the rename tool doesn't need to care about preserving the old path and can always recompute the relative path itself.

No such step needs to be implemented, but I agree that it's ok for the operation to be lossy in this way.

  • Nix paths turn /foo/bar/../baz into /foo/baz without checking for what bar is (could be a symlink). This is however rare enough that it isn't really worth looking into
  • Such a rename tool can notably still cause evaluation changes, though this should be rare:

It's rare for renames to be entirely undetectable, being guaranteed to work. Nix has toString. Many languages have reflection.
Yet, renaming tools exist in IDEs for most languages.

  • Renames should be reversible, nix-file-rename a b && nix-file-rename b a is a no-op

This contradicts the earlier idea that implicitly canonicalizing path expressions is ok. We can restrict the reversibility property to expressions with canonical paths only.

  • Note that not only need file references to be tracked in the index but also folders, since you can reference just folders (and move entire folders)

The renames are a function between paths. Operations on this function should take into account the hierarchical nature of paths, but do not need to know the types of paths as could be learned from lib.pathType or stat(2). If we allow implicit canonicalization, the tool can use the same representation that Nix uses internally: absolute paths.

@infinisil
Copy link

I have started with something like this here: nixpkgs-architecture/simple-package-paths#22. It doesn't implement the renaming yet, but only the first step towards that, I plan to work more on this

@abhillman
Copy link

I too am interested in this. The thing I am getting stuck on -- how can I create an ast node from another node while changing some of its values? Would someone be able to provide any example code? I will keep playing around to see if I can get this, but am a bit lost as to this at the moment (especially because, afaict, methods that would allow this seem to be private).

@abhillman
Copy link

abhillman commented May 7, 2024

I too am interested in this. The thing I am getting stuck on -- how can I create an ast node from another node while changing some of its values? Would someone be able to provide any example code? I will keep playing around to see if I can get this, but am a bit lost as to this at the moment (especially because, afaict, methods that would allow this seem to be private).

Perhaps related, which doesn't require private calls. Taken via https://github.com/nix-community/rnix-parser/blob/master/src/ast/str_util.rs#L273.

fn main() {
    let content = "cool";

    let mut builder = GreenNodeBuilder::new();
    builder.start_node(NixLanguage::kind_to_raw(NODE_STRING));
    builder.token(NixLanguage::kind_to_raw(TOKEN_STRING_START), "''");
    builder.token(NixLanguage::kind_to_raw(TOKEN_STRING_CONTENT), content);
    builder.token(NixLanguage::kind_to_raw(TOKEN_STRING_END), "''");
    builder.finish_node();

    let zzz: rnix::ast::Str = ast::Str::cast(SyntaxNode::new_root(builder.finish())).unwrap();
    println!("{}", zzz);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants