Skip to content

[2.x] feat: queriable slash syntax (sbt query)#7699

Merged
eed3si9n merged 4 commits intosbt:developfrom
eed3si9n:wip/query
Oct 2, 2024
Merged

[2.x] feat: queriable slash syntax (sbt query)#7699
eed3si9n merged 4 commits intosbt:developfrom
eed3si9n:wip/query

Conversation

@eed3si9n
Copy link
Copy Markdown
Member

@eed3si9n eed3si9n commented Sep 26, 2024

See https://eed3si9n.com/sudori-part6/ for details.

Fixes #7642

Problem

We want a more flexible way of aggregating subprojects.

Solution

This implements a subproject filtering as a replacement of the subproject axis in the act command.

sbt:root> print b.../name
bar / name
  bar
baz / name
  baz

sbt:root> ...@scalaBinaryVersion=3/test
[info] Passed: Total 0, Failed 0, Errors 0, Passed 0
[info] No tests to run for foo / Test / testQuick
[info] compiling 1 Scala source to target/out/jvm/scala-3.3.1/root/backend ...
[info] compiling 1 Scala source to target/out/jvm/scala-3.3.1/root/test-backend ...
[info] Passed: Total 0, Failed 0, Errors 0, Passed 0
[info] No tests to run for Test / testQuick
[success] elapsed time: 3 s, cache 58%, 7 disk cache hits, 5 onsite tasks

**Problem**
We want a more flexible way of aggregating subprojects.

**Solution**
This implements a subproject filtering as a replacement of
the subproject axis in the act command.
Copy link
Copy Markdown
Member

@adpi2 adpi2 left a comment

Choose a reason for hiding this comment

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

Before going into the implementation I'd like to discuss the syntax.

I had never seen this ...@foo=bar syntax before #6801. Is it specified anywhere? Is it used in other tools?

How is it better than a regex or a glob pattern?

Since the binary version and platform are contained in the project name we could use a glob pattern like *_sjs1_3 instead of ...@binarScalaVersion=3@platform=sjs1. It is more concise, seems more flexible (custom axes) and should look more familiar to most people.

@eed3si9n
Copy link
Copy Markdown
Member Author

Bazel query uses ... to mean all. Here I am using it as glob. So ..._sjs1_3 would work. The benefit of using . is that it works on shells like Zsh without quoting whereas shell would probably replace * into some file name.

@adpi2
Copy link
Copy Markdown
Member

adpi2 commented Sep 26, 2024

Do we really need the @binaryScalaVersion=3 then? Should it support any SettingKey[String] or only a predefined set of keys?

@eed3si9n
Copy link
Copy Markdown
Member Author

Do we really need the @binaryScalaVersion=3 then?

For this specific usage I don't think we need it, but I think it becomes complicated to remember which suffix comes first etc for the build users, so the explicit parameter looks nicer on GitHub Action etc.

Should it support any SettingKey[String] or only a predefined set of keys?

I'm not exactly sure how safe it would be from the parser to call arbitrary keys, but it might work? In this PR I just wanted to add something as a demo.

@alexcardell
Copy link
Copy Markdown

alexcardell commented Sep 26, 2024

This is potentially pendantic and a bit late but it is possible to use a more terse syntax for the glob? Either a single ., or _. I would like to reduce the amount of characters I'm typing 100x a day. Plus I feel ... indicates that you're globbing over three dimensions

I quite like _, it feels like the wildcard in Scala. SBT seems to parse it fine already, e.g. if I try sbt _/test

Cool idea for filtering though! I've been missing that using sbt-typelevel projects which use crossproject

@eed3si9n
Copy link
Copy Markdown
Member Author

eed3si9n commented Sep 26, 2024

@alexcardell

This is potentially pendantic and a bit late but it is possible to use a more terse syntax for the glob?

Typo on pedantic :). But, in general, I welcome feedback on the query syntax at this point in time.

Either a single ., or _. I would like to reduce the amount of characters I'm typing 100x a day. Plus I feel ... indicates that you're globbing over three dimensions

  1. Once you get used to it ... is very easy to type. Bazel users do this all day every day.
  2. I'd push back a bit on this notion of "command must be short". Early versions of sbt shell syntax optimized on tab completion and used mix of /, :, ::. This was very efficient for 1% of the wizards who memorized how this worked, but confused 99% of the user base. Also with modern shell environment with history, you're more likely than not just typing up and down arrows.

I quite like _, it feels like the wildcard in Scala. SBT seems to parse it fine already, e.g. if I try sbt _/test

There are multiple problems with the underscore (_) that I can think of.

  1. First, _ is a legal Scala identifier character, which is used as the subproject name that we want to filter out, including the ones generated by the projectmatrix.
  2. Second, I feel like the Scala semantics of _ (let's say the placeholder syntax _ + 1) muddies the sbt query since it's not a one-to-one match. We're passing in a filter argument, not defining a lambda. The following is allowed:
print b...a.../name

but

print b_a_/name

Cool idea for filtering though! I've been missing that using sbt-typelevel projects which use crossproject

Thanks!

@mrdziuban
Copy link
Copy Markdown
Contributor

This is a great idea! Having used mill a bit I really liked its wildcard syntax and missed it when I came back to sbt.

That said, I'd like to add my support for a shorter syntax as well.

Either a single ., or _. I would like to reduce the amount of characters I'm typing 100x a day. Plus I feel ... indicates that you're globbing over three dimensions

  1. Once you get used to it ... is very easy to type. Bazel users do this all day every day.

The way I see it, it's not so much about being easy to type as it is about how quickly it can be typed. There's no way that typing ... will ever be faster than just typing .

  1. I'd push back a bit on this notion of "command must be short". Early versions of sbt shell syntax optimized on tab completion and used mix of /, :, ::. This was very efficient for 1% of the wizards who memorized how this worked, but confused 99% of the user base. Also with modern shell environment with history, you're more likely than not just typing up and down arrows.

I don't 100% follow how the mix of symbols relates to short commands. If the concern is that adding new symbols will confuse users then it doesn't matter how short the new syntax is, it will confuse people regardless. IMO the shorter the syntax, the easier to learn/remember.

There are multiple problems with the underscore (_) that I can think of.

I totally get the concern with _, its meaning in Scala is already overloaded (though less so in Scala 3).

I understand the concern with * re: shell quoting, but it would be my preference. I pretty much always run commands within the sbt shell, and in the off chance that I run something from bash/zsh then I don't mind having to quote. Related to the earlier points as well, I think * is least likely to confuse users as many will already be familiar with its meaning in glob patterns and can identify the connection to what it's doing in sbt.

Would you consider supporting * in addition to another syntax?

@eed3si9n
Copy link
Copy Markdown
Member Author

@mrdziuban

Would you consider supporting * in addition to another syntax?

Yea. I am warming up to *. If we do support *, then I don't know if we should support both ... and *. That seems like we're inviting confusion.

@eed3si9n
Copy link
Copy Markdown
Member Author

A bit of the note on sbt shell vs system shell:

I pretty much always run commands within the sbt shell, and in the off chance that I run something from bash/zsh then I don't mind having to quote.

Using the sbt shell has long been our recommendation (to keep the JIT warm), but with the advent of BSP support, I've switched to using the native client sbtn or sbt --client instead to so sbt session can be shared with Metals. If we make that the default for sbt 2.x, people should be free to use their system shell and call sbt .../test. Also query would probably be useful on CI to split Scala.JS into different worker etc.

Given that many projects would have src/test directory, especially for their hello world projects:

$ sbt */test

actually ends up matching

$ sbt src/test

So if we supported *, a common error a build user might run into is:

[error] Expected ID character
[error] Not a valid command: src (similar: set)
[error] Expected <unspecified>
[error] Expected '@scalaBinaryVersion='
[error] Expected project ID
[error] Expected configuration
[error] Expected ':'
[error] Expected key
[error] Not a valid key: src (similar: sources, ps, run)
[error] src/test
[error]    ^

So I feel like this is a tricky UX design. If we make something intuitive, but it has a major pitfall, we're basically making an on-ramp into a pitfall, and future work for ourselves.

Copy link
Copy Markdown
Contributor

@BillyAutrey BillyAutrey left a comment

Choose a reason for hiding this comment

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

🚢

Here's my opinion on the choice of wildcard. Full disclosure, I have spent the last few years in the Bazel space.

I think ..., _, and * are the only real wildcard contenders. Everything else is not intuitively a wildcard.

  • * breaks shell integration. We want to embrace sh as a viable option for SBT interaction, because the new --client option allows us to amortize the JVM warmup cost. Thus, to use *, we'll be forcing shell users to escape with quotes.
  • _ has compatibility issues, which Eugene has detailed above. We want to preserve compatibility with existing projects.
  • ... is not intuitive at first, but it's easy to learn and is acceptable to thousands of Bazel users. And having used it for 3+ years now, it doesn't bother me.

I see build tool interactions homogenizing a bit, and ... is an acceptable and familiar wildcard for many build tool users. I say we go with it.

@eed3si9n eed3si9n merged commit 133b737 into sbt:develop Oct 2, 2024
@eed3si9n eed3si9n deleted the wip/query branch October 2, 2024 06:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants