Page MenuHomePhabricator

How to integrate Arcanist with less granular build systems
Closed, WontfixPublic

Description

Hi, I'm wondering if there's any ideas for how to support other build systems which don't support running linting or unit tests at the same level of granularity as CLI tools.

We use gradle for a groovy project, and in general I run: ./gradlew check which generates both lint and unit test reports whose output I'd like to collect on arc diff.

In essence, would it be possible to do something like:

  1. run arc diff
  2. This would first do ./gradlew check
  3. linter collects checkstyle output
  4. unit test runner collects xUnit output

Event Timeline

rymndhng renamed this task from How to integrate Arcanist with bigger build system to How to integrate Arcanist with less granular build systems.
rymndhng raised the priority of this task from to Needs Triage.
rymndhng updated the task description. (Show Details)
rymndhng added a project: Arcanist.
rymndhng added a subscriber: rymndhng.

Essentially, the issue here is that tools such as gradledont have any east way to run linters/tests on a subset of the code base. It's possible to write a linter which wraps around ./gradlew check but this means that arc lint -- file takes the same amount of time and resources as arc lint --everything .

You would be better off setting up the linters / unit tests through arc, and then making gradle run arc lint and arc unit as part of it's process.

You would be better off setting up the linters / unit tests through arc, and then making gradle run arc lint and arc unit as part of it's process.

To do this we either need to know where gradle installs the tools (I tried figuring it out, its overly complex) or install the tools globally instead.

epriestley claimed this task.
epriestley added a subscriber: epriestley.

I guess you can conceivably use some static property somewhere to coordinate a lint engine and unit test engine in a hacky-but-functional way.

e.g, both of them call HackyCoordinator::runGradleIfNotAlreadyRun(), which looks like:

static $already_run = false;
if (!$already_run) {
 run_gradle();
 $already_run = true;
}

Then the LintEngine looks at the checkstyle output file and the UnitEngine looks at the xunit output.

This will run gradle once per arc diff instead of twice, at the cost of being kind of real gross. This should work fine, though, technically speaking.

It won't solve the problem that arc lint -- file and arc unit will be a lot slower than they would be if things were granular, but that obviously isn't really solvable if no command exists which can run only part of the work.

At least personally, I think these unit/lint checks in arc diff are most valuable if they run in <60 seconds (and, ideally, <15 seconds). My very limited experience with Gradle (building Minecraft mods) suggests it can not execute an empty buildfile in less than about 15 minutes, so I'd possibly consider not integrating anything into arc diff and just doing everything in CI, maybe, if these are the kind of timescales you're looking at. A different balance of tradeoffs may make more sense in your environment, of course.

We could provide some HackyCoordinator helper in the upstream, but I don't think it's really worthwhile since we can only make the behavior slightly less bad. We do sort of have half a mechanism like this in the linters right now (used to share syntax trees between XHPAST linters) and we could theoretically generalize it to be shared between lint/unit engines, but I don't think this is a very good use case for motivating that since the meat of the problem is basically intractable. I think this situation of having unit and lint completely coupled into the same command is relatively unusual, too.

Ack, our timescale is in the order of ~30 in WCET so it's not too terrible (it's a tiny repo). But I'd definitely prefer sub 15. As it stands at the moment, we may be able to go with simply running all the tests.

I just found this bug because I have exactly the same problem and would love to see this fixed.

My very limited experience with Gradle (building Minecraft mods) suggests it can not execute an empty buildfile in less than about 15 minutes, so I'd possibly consider not integrating anything into arc diff and just doing everything in CI, maybe, if these are the kind of timescales you're looking at. A different balance of tradeoffs may make more sense in your environment, of course.

For us, part of the motivation is that things are much faster if run through gradle. If you use the gradle daemon it's up and running all the time and builds are extremely fast. We're writing Java and want to do our style checks with Checkstyle. arcanist doesn't have a checkstyle linter so you have to set that up with the script-and-regex linter. Since Checkstyle is a Java app that means that a diff with 10 files requires 10 JVM starts and stops which is very slow while the gradle alternative is essentially instant (far less than a second).

The best solution I've come up with so far is to invert things and create a gradle task that runs checkstyle and then calls "arc diff". That works but seems hacky.

I think there's lots of similar use cases where somebody would like to run some arbitrary command before and/or after creating a diff. If the command returns a non-0 exit code the diff creation should fail. So a simple run.before property we could put in our .arcconfig would solve this problem and, I imagine, have other uses as well (e.g. could be used to post to slack that a new diff was created).

Please consider re-opening this bug.

run some arbitrary command before and/or after creating a diff

That's supported with arc alias:

arc alias moo '! something && arc list && something else'

The trouble with running lint/unit outside of arc is that the information isn't uploaded to the server.

Thanks for the thought. There's 2 issues with this solution. The first is, as you mention, that the result doesn't get uploaded to the server. The 2nd is that there's no enforcement: developers can still run "arc diff" to create a diff that has lint errors and nobody would know. What I'm looking for is a way to efficiently run checkstyle for each new diff. Ideally the results of the checkstyle run would be sent to the server but I'd settle for a solution that simply gives the dev and error message and bails.

There's more discussion of this in T9223, which effectively superseded this. I've filed T10329 to describe the pathway forward.

If you are are extremely adventurous, you could make a checkstyle-daemon, some start-daemon-or-use-if-already-running command (Maybe gradle in daemon mode can already do something like that?), and a custom ArcanistLinter that talks to it. That's a significant amount of work, but does not clash with arc's current world view.

Also, if whatever you're running is extremely fast already (i.e., effectively caching results for you), you can just have both lint and unit run it and ignore the parts of the output they don't care about.

T9223 has another flavor of this, using static properties or singletons or whatever to share the data between the workflows.

(I'm not sure if the daemon is fast like "100 milliseconds" or fast like "5 seconds").