Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Campaigns: src actions schema: add ability to reference config file from Docker Action #10527

Closed
beyang opened this issue May 10, 2020 · 5 comments
Assignees
Labels
batch-changes Issues related to Batch Changes estimate/1d src-cli

Comments

@beyang
Copy link
Member

beyang commented May 10, 2020

Here's the scenario: I would like to use Campaigns with Comby to initiate a refactor. The Comby transformation involves rewrite rules. I used https://comby.live to compose it:

image

It converts log.Printf calls like this:

log.Printf("Add %v as a temporary child of root %v", t.Span.ID, root.Span.ID)

into log15.Warn calls like this:

log15.Warn("Add as a temporary child of root", "t.Span.ID,", t.Span.ID, "root.Span.ID", root.Span.ID)

I would like to use the comby/comby Docker container as an Action in the Campaign. To do so, I need to specify Action JSON like the following:

{
  "scopeQuery": "repo:^github\.com/sourcegraph/ log.Printf",
  "steps": [
    {
      "type": "docker",
      "image": "comby/comby",
      "args": [...]
    }
  ]
}

However, specifying the Comby transformation entirely as command-line args will be quite verbose. And when I click the "copy" button on https://comby.live, it gives me a blob of shell script to run:

COMBY_M="$(cat <<"MATCH"
log.Printf(":[format]", :[args])
MATCH
)"
COMBY_R="$(cat <<"REWRITE"
log15.Warn(":[format]", :[args])
REWRITE
)"
COMBY_RULE="$(cat <<"RULE"
where
rewrite :[format] { "%:[[_]] " -> "" },
rewrite :[format] { " %:[[_]]" -> "" },
rewrite :[args] { ":[arg.]" -> "\":[arg]\", :[arg]" }

RULE
)"
# Install comby with `bash <(curl -sL get.comby.dev)` or see github.com/comby-tools/comby && \
comby "$COMBY_M" "$COMBY_R"  -rule "$COMBY_RULE"  -stats

I would like to run this shell script in the comby/comby Docker container as the Action in my Campaign. To do this, it seems like we need 2 additions to the src actions CLI:

  • Mount the current working directory on the host into some directory in the container, so it can be referenced
  • Add a entrypoint field to the Actions JSON schema, so I can override the entrypoint, as well as pass arguments to the default entrypoint.

These 2 things would let me create the shell script file (what I copied from https://comby.live) on my host machine and then reference it from the Comby Action (which is a Docker-based Action).

An obvious workaround is to use a Command-type Action, but it seems like there might be cases where the user doesn't have the command available on their host machine and wants to use a Docker Action while passing in a configuration file from their host FS to that container.

@beyang beyang changed the title Campaigns: ability to pass config file to Docker Action Campaigns: src actions schema: add ability to reference config file from Docker Action May 10, 2020
@sqs sqs assigned sqs and unassigned mrnugget May 11, 2020
@sqs
Copy link
Member

sqs commented May 11, 2020

Reassigning to me as the PM until we prioritize this.

You could also create your own Docker image to do this. This may be obvious, but you want to avoid that and want an easier way to run Comby campaigns, right?

@beyang
Copy link
Member Author

beyang commented May 11, 2020

Writing a Dockerfile feels like it’s too much overhead for something that seems like it’ll be relatively common. Also some users might not be familiar with Docker builds.

@sqs sqs removed their assignment Jul 18, 2020
@sqs sqs added the batch-changes Issues related to Batch Changes label Jul 18, 2020
@mrnugget
Copy link
Contributor

@LawnGnome I think with the new YAML format there might be a way to provide some syntactic sugar and/or pre-defined Docker images to accomplish this easily.

@LawnGnome
Copy link
Contributor

This ties into the discussion we had last week around what run means in the YAML spec: if we do end up providing entrypoint and cmd keys as another way of specifying how to run the container, that gets us most of the way to dealing with this issue. (That said, I'm a little worried that we're going to end up providing an ad hoc implementation of Docker Compose.)

@mrnugget
Copy link
Contributor

mrnugget commented Oct 29, 2020

With the new templating and file mounting support in campaign specs added as an experimental feature in sourcegraph/src-cli#361 this would be possible like this:

name: comby-log-printf
description: |
  This campaign uses [Comby](https://comby.dev) to rewrite `log.Printf` calls with `log15.Warn` calls.

on:
  - repositoriesMatchingQuery: lang:go log.Printf(":[format]", :[args]) patternType:structural repo:sourcegraph$

steps:
  # Run comby only over the exact search result paths:
  - run: comby -in-place -matcher .go -config /tmp/comby-conf.toml -f ${{ join repository.search_result_paths "," }}
    container: comby/comby
    files:
      # Create files inside the container by specifying path and content here:
      /tmp/comby-conf.toml: |
        [log_to_log15]
        match='log.Printf(":[format]", :[args])'
        rewrite='log15.Warn(":[format]", :[args])'
        rule='where
        rewrite :[format] { "%:[[_]] " -> "" },
        rewrite :[format] { " %:[[_]]" -> "" },
        rewrite :[args] { ":[arg~[a-zA-Z0-9.()]+]" -> "\":[arg]\", :[arg]" }'

  # Now run goimports, but only over those files that comby modified:
  - run: goimports -w ${{ join previous_step.modified_files " " }}
    container: unibeautify/goimports

changesetTemplate:
  title: Replace log.Printf with log15.Warn
  body: |
    This campaign replaces `log.Printf` calls with with `log15.Warn` to standardize on a logger.

    It converts `log.Printf` calls like this:

    ```
    log.Printf("Add %v as a temporary child of root %v", t.Span.ID, root.Span.ID)
    ```

    into `log15.Warn` calls like this:

    ```
    log15.Warn("Add as a temporary child of root", "t.Span.ID,", t.Span.ID, "root.Span.ID", root.Span.ID)
    ```
  branch: campaigns/sprintf-to-itoa
  commit:
    message: Replacing fmt.Sprintf with strconv.Iota
    author:
      email: [email protected]
      name: Thorsten Ball
  published: false

Note, though, that I can't get the comby pattern to work for complex pattern where the :[arg] is a function call that includes strings etc. See comby.live test here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
batch-changes Issues related to Batch Changes estimate/1d src-cli
Projects
None yet
Development

No branches or pull requests

5 participants