west
west copied to clipboard
zephyrproject-rtos
New "west grep" shortcut for west forall -c 'git grep <pattern>'
git has a built-in git grep command that is convenient to perform a recursive search in sources. It's very convenient because
- it automatically excludes binary
.git/database files and built artefacts, - it works well "out of the box" requiring zero configuration or hard to remember option or parameter
- it supports
grep's standard and familiar user interface / syntax - it's built-in requiring zero additional installation
- it's extremely short to type
- it's seems faster than the equivalent recursive grep . Maybe because it's multithreaded by default?
In a multi repo project, developers sometimes need to search across repos. Either because the project has (too) many repos and they need a reminder, or because some symbol is actually used across multiple repos, or because <share other reasons below>.
west forall -c 'git grep <pattern>' is then the obvious solution, however it loses a lot of the convenience listed above and introduces additional and non-portable quoting complexity.
It's probably why Grepo already has the repo grep shortcut.
@mbolivar on Slack:
I would like people to start asking for features on GitHub, and voting with emoji.
I've recently learned that a feature I've been working on for a long time is not expected to be useful to the people I've been making it for, and there are a lot of directions we can go with west, I want to start making some sort of priority queue based on user feedback I want to be clear that I'm happy to discuss features in here [on Slack], but I'd like to start asking for more issues filed for new features before I at least start implementing. There are already quite a few enhancements open (edited) And TBH I think it's rather easy to write a shell function to wrap your favorite grep-a-like, so without more people asking for it, I'm not inclined to make it a built-in command
In a different context:
just try to be conservative about adding [top-level built-in commands] unless they're really needed.
Maybe west grep could be an optional extension that users download and install from some random place on github ? For convenience and to avoid duplication of the simplest "user's choice". I don't know anything about west extensions so not sure I'm making sense.
My position, from slack:
- There is an easy workaround in place, namely defining a shell function that calls
west forallwith your favorite git-type program, so I don't think this is a blocker (we've spent way more time talking about this than it would have taken to write that shell function) - I'm not happy with how our other existing commands that are basically
west forallwrappers, namelywest diffandwest status, work, and I don't want to add more without solving some problematic pieces of their behavior - As you quoted, I want to start using issues + emojis as a way to let people vote for what is important to them. More votes --> more important
Details on 2.:
The west diff and west status helpers never worked the way I wanted them to and I'm not happy with them. This is the problem:
https://github.com/zephyrproject-rtos/west/issues/242
The way I "solved" this problem in the commit which closed the issue was by redefining these commands so you can't pass any arguments to git diff or git status. I'm not happy with this "solution", and it makes me wary of adding new commands that are essentially west forall wrappers without something better.
After all, git diff, git status, and git grep all take options, and people are going to want to use them.
I don't like this alternative solution either:
west diff [ --git-diff-arg=will-be-passed-to-git-diff ] [PROJECT ...]
because it's too verbose (even with a short form for --git-diff-arg).
I guess we could just make the commands accept any and all unknown arguments, and manually look for --. I think since I ran into west #242, that sort of thing was added to the west build extension that's part of zephyr. But I haven't had time to try replicating it in west itself.
Maybe someone can help out here? I would appreciate it.
west diff [ --git-diff-arg=will-be-passed-to-git-diff ] [PROJECT ...]
The problem with solutions like --git-diff-arg is always the same: nesting makes quoting much more challenging because it messes with the levels of interpretation. Let's say you want to pass to git diff some argument(s) with whitespace, I mean something like this:
git diff HEAD~10 -- -U 20 'some directory name'
Good luck passing this to --git-diff-arg
C compilers have two solutions I'm aware of, none convenient for simpler cases IMHO:
-
cc -Xlinker HEAD~10 -Xlinker -- -Xlinker -U -Xlinker 20 -Xlinker 'some directory name' -
cc -Wl,HEAD~10,--,-U,20,'some directory name'# don't start any filename with a comma?
I guess we could just make the commands accept any and all unknown arguments, and manually look for --.
This made me curious so I tried to break --:
printf '%s' '-e\n' > master
git grep --untracked -- -e -- master # master is here the file, not the branch!
It Just Works! Until now I never realized the magical nesting properties of --... a silver bullet?
How about a generic west forallgit diff repo1 repo2 -- diffargs where forallgit passes any git command through?
I wonder whether grepo and others ever considered that.
This got four +1 emojis and one -1 so far. Thanks for voting, everyone. It seems like the general reaction is positive.
The problem with solutions like --git-diff-arg is always the same: nesting makes quoting much more challenging because it messes with the levels of interpretation.
Yep, I'm a strong -1 on --git-diff-arg type options.
How about a generic
west forallgit diff repo1 repo2 -- diffargswhereforallgitpasses any git command through?
So the signature would be something like:
west forallgit GIT_COMMAND [PROJECT ...] -- [GIT_COMMAND_ARGS]
and the semantics would be equivalent to
west forall -c 'GIT_COMMAND GIT_COMMAND_ARGS' [PROJECT ...]
??
Would I be correct to say that providing a single west forallgit, plus a west alias command that would let you run
west alias grep 'forallgit grep'
once, and then subsequently west grep would mean the same thing as west forallgit grep, might be satisfactory?
I wonder if there's a more generic way to provide a "forall alias" type function that doesn't assume git.
once, and then subsequently west grep would mean the same thing as west forallgit grep, might be satisfactory?
I think it would be really cool.
I wonder if there's a more generic way to provide a "forall alias" type function that doesn't assume git.
Well, git is very special to west isn't it? Far beyond the "favorite child"!
I wonder if there's a more generic way to provide a "forall alias" type function that doesn't assume git.
Well, git is very special to west isn't it? Far beyond the "favorite child"!
Indeed, but I think it's worth it. For example, as discussed on slack, I prefer ripgrep to git grep, so it would make my life better to have a more generic way to do this that doesn't assume git.
It's not just my weird regular expression utillity preferences, either. There are other commands, like find (or fd), that people might want "forall aliases" for. I know about git ls-files, but muscle memory for find dies hard too.
I came here searching for a way to create aliases for particular, frequently used west commands, preferable they would somehow be stored in the manifest. For example:
self:
path: somepath
west-commands: west-commands.yml
west-aliases:
- grep: "forall -c 'git --no-pager grep -I $1'"
This would allow projects to easily define their own aliases for whatever task they need it for without the need to anticipate use cases in separate extensions. The benefit with having the aliases in the manifest / config is that it's self documenting and doesn't require extra configuration scripts to be distributed among the team.
This would allow projects to easily define their own aliases for whatever task they need it for without the need to anticipate use cases in separate extensions.
I don't understand this argument, sorry. I don't see anything you can do in this alias example that you can't do in an extension.
Wasn't probably clear enough, and this is an issue to add west grep support, so the assumption probably is to create a new extension. The argument I was trying to make wasn't what you can do with an extension but rather what you should do. Why would want to make a python class to run a oneliner, even if you could? You don't need to write a new script to create a git command alias either, but you could. Universally working west grep extension that would do the right thing by default (west grep foobar) would be really nice, but given an arbitrary project list it's hard to say what is the correct behavior. It is possible to create shell aliases for those cases, but that probably causes people within the same team to reinvent the wheel in their own workspaces. Also, what about similar simple tasks which might require long list of default options, e.g. updating project tags files?
So if there's going to be a west grep extension, hooray! However, I would like there to be command alias support as well.
For the record, I'm using west on a non-Zephyr project and we've created multiple extensions to support our workflow better. This grepping annoyance doesn't seem like the thing we'd spend time / effort to create an extension for, hence the desire for aliases.
The benefit with having the aliases in the manifest / config is that it's self documenting and doesn't require extra configuration scripts to be distributed among the team.
What's wrong with distributing scripts using the same git repo as the manifest? = what Zephyr does for its west extensions and more. Wait, isn't that what every project does?
Why would want to make a python class to run a oneliner, even if you could?
To... avoid bloating the manifest with things totally unrelated to version control?
If creating and sharing west extensions is too much hassle then maybe that should be reported and addressed?
What's wrong with distributing scripts using the same git repo as the manifest? = what Zephyr does for its west extensions and more. Wait, isn't that what every project does?
To... avoid bloating the manifest with things totally unrelated to version control?
Aren't the extensions (west-extensions.yml) already totally unrelated to version control? I tend to think them as related to the project and the workflows used within the project. What I was suggesting was merely syntactic sugar to allow distributing project-specific west "extensions" without the need for writing a python class for it. I have no objections for the aliases being in the extension file instead of the main manifest.
If creating and sharing west extensions is too much hassle then maybe that should be reported and addressed?
It's not too much hassle for all cases, but wrapping a single command in a python class is close to the definition of boilerplate. Maybe if west supported using shell scripts (or arbitrary binaries like git) as extensions this issue wouldn't exist. IMO It would still be uglier than aliases, but for simple command wrappers it would be pretty decent.
Aren't the extensions (west-extensions.yml) already totally unrelated to version control?
West is not just version control but the manifest is 100% version control (so far).
I have no objections for the aliases being in the extension file instead of the main manifest.
Thx I think we're on the same page.
Maybe if west supported using shell scripts (or arbitrary binaries like git) as extensions this issue wouldn't exist. IMO It would still be uglier than aliases, but for simple command wrappers it would be pretty decent.
Sounds good to me. @mbolivar-nordic ?
Maybe if west supported using shell scripts (or arbitrary binaries like git) as extensions this issue wouldn't exist. IMO It would still be uglier than aliases, but for simple command wrappers it would be pretty decent.
Sounds good to me. @mbolivar-nordic ?
I've thought about this in the past and I agree it would be interesting to support this. However, you would need to address how to get help for extensions in output for things like west help my-extension and plain west help.
I would be happy to have a design discussion and review patches if you are proposing to implement this yourself, @lindblandro
Frankly, I'm hesitant to commit as I'm unsure if I have the time required and I currently have very vague idea how large this task would be. If you (@mbolivar-nordic) can provide some gentle guidance and don't expect the implementation to be ready ASAP then I can maybe give it a try.
@lindblandro this is an open-source project; there is no deadline here. People may have west-related deadlines inside their own company but they are not tracked in this Github project. If this were a complex feature then you could be asked to "commit" to long term maintenance if any is required but only after it's reviewed, accepted and merged. Hopefully this won't be very complex feature anyway.
@mbolivar-nordic already offered some of his limited time to review a tentative design and/or prototype code so if you have some ideas of how this should work then please refine and share these design ideas in a new issue (I don't remember any existing issue matching this). It's OK if it does not go very far; someone else with more time or having a more urgent need would still be happy to find that discussion in a few years time.
Sounds good, I'll think of a couple use cases at work and have a look at the code and open a new issue.