git.origin -> git.github_pr_destination fails when specifying a SHA-1 `ref` on the origin

sudoforge commented 4 years ago

I'm attempting to write a macro for a workflow which will take a public Git SoT (git.origin is necessary, since this can be sources other than github or gerrit). I'd like to submit this import as a PR to a private github destination (via git.github_pr_destination).

The workflow that gets generated is pretty simple:

    core.workflow(
        name = "some_foo_name",
        mode = "SQUASH",
        origin = git.origin(
            url = url,
            ref = "someCommitHash",
        ),
        destination = git.github_pr_destination(
            url = private_url,
            destination_ref = "master",
            pr_branch = "import/some_foo_name/${CONTEXT_REFERENCE}",
        ),
        <snip...>

However, when running this workflow, I get a error stating that I need to specify the pr_branch for this:

ERROR: git.github_pr_destination is incompatible with the current origin. Origin has to be able to provide the contextReference or use '--github-destination-pr-branch' flag

Am I going about this incorrectly? What is a more idiomatic way to do this?

sudoforge commented 4 years ago

I should note that providing the option on the command line works, of course, but then I lose the ability to populate the pr_branch value dynamically (in the macro, some_foo_name is actually a variable).

sudoforge commented 4 years ago

Okay, I feel silly. This was due to not providing a ref when invoking the workflow on the command line. For example, I was calling this with:

copybara migrate path/to/copy.bara.sky --init-history

When I should have been providing a ref, as in:

copybara migrate path/to/copy.bara.sky <someRef> --init-history

This feels like a bug, because ref is documented as a parameter for git.origin and I would assume that should work, regardless of whether or not that is provided on the command line or explicitly in the workflow.

sudoforge commented 4 years ago

For context, what I'm trying to do is codify the import of third party dependencies (go modules). The full macro is:

load("//<snip>", "private_url")
load("//<snip>", "relative_glob")

def go_module(
        name,
        path,
        url,
        ref = "master",
        files = glob([]),
        patches = [],
        license = None,
        transformations = []):
    core.workflow(
        name = name,
        mode = "SQUASH",
        origin = git.origin(
            url = url,
            ref = ref,
        ),
        destination = git.github_pr_destination(
            url = private_url,
            destination_ref = "master",
            pr_branch = "import/" + name + "/" + ref,
        ),
        origin_files = glob(
            include = files + [
                "*LICENSE*",
            ],
            exclude = [
                "**_test.go",
                "go.mod",
                "go.sum",
            ],
        ),
        destination_files = relative_glob(
            path,
            include = ["**"],
            exclude = [
                "BUILD",
                "copy.bara.sky",
            ],
        ),
        authoring = authoring.overwrite(
            default = "Copybara Bot <no-reply@sudoforge.com>",
        ),
        transformations = transformations + [
            core.move("", path),
            <snip>
        ],
    )

And then in //third_party/go/copy.bara.sky, use this module to pin external dependencies like so:

load("//<snip>", "go_module")

go_module(
    name = "com_github_spf13_cobra",
    url = "https://github.com/spf13/cobra",
    ref = "11ba63fc3bc37818c471fc7912783e6909da4645",
    path = "third_party/go/github.com/spf13/cobra",
    license = "notice",
    files = [
        "*.go",
    ],
)

go_module(
    name = "com_github_inconshreveable_mousetrap",
    url = "https://github.com/inconshreveable/mousetrap",
    ref = "76626ae9c91c4f2a10f34cad8ce83ea42c93bb75",
    path = "third_party/go/github.com/inconshreveable/mousetrap",
    license = "notice",
    files = [
        "trap_others.go",
        "trap_windows.go",
    ],
)

<snip>

The ultimate goal here is to build a service that will run copybara migrate third_party/go/copy.bara.sky <workflow name> [--init-history] after commits to master. The current limitations to making this smooth are:

Unable to programmatically enumerate all of the workflows in a given copy.bara.sky
Unable to utilize the specified ref parameter of the macro
Having to manually generate the BUILD files (so they are included and updated as part of the migration) instead of invoking an external tool like bazel-gazelle to do this for me

The main motivations behind approaching dependency management in this manner is to:

Codify all of my dependencies, including things that aren't Go modules (e.g. bazel rules, external tools, etc) using a common interface, e.g. some set of macros that generate various purpose-focused Copybara workflows
Using Copybara as the single method of moving source code into and out of my private repository

Having to manually generate the BUILD files (so they are included and updated as part of the migration) instead of invoking an external tool like bazel-gazelle to do this for me

One solution I've thought of for this would be a feature request to be able to execute a Bazel target in the origin as a transformation. I'm not 100% sure this would be efficient given the differing architecture of Copybara and Bazel (since Bazel would run in the temporary working directory during the migration), but consider it a wishlist item nonetheless.

I'm currently thinking about having the aforementioned service run the migration, and then checkout and update the destination reference as needed. This is completely doable, but a little clunky.

sudoforge commented 4 years ago

Actually, it looks like the sha reference may not be getting parsed correctly, or I'm not providing it correctly:

➜ copybara migrate third_party/go/copy.bara.sky com_github_spf13_cobra 11ba63fc3bc37818c471fc7912783e6909da4645 --init-history
May 09, 2020 10:21:41 AM com.google.copybara.Main configureLog
INFO: Setting up LogManager
Copybara source mover (Version: Unknown version)
Task: Git Origin: Initializing local repo
ERROR: git.github_pr_destination is incompatible with the current origin. Origin has to be able to provide the contextReference or use '--github-destination-pr-branch' flag

I think this would also impact the ref property, and could be the reason the generated workflow isn't working as I expect.

sudoforge commented 4 years ago

I've tested the above macro using a branch name, and it works without specifying a ref on the command line, so the issue here is that I can't set the ref property of git.origin to a SHA-1. Based on the documentation, this appears to be a bug.

hsudhof commented 4 years ago

Will look into fixing the documentation; pr_origin expects a PR as ref, e.g. a PR number or its branch.

Cheers, ~H

On Sat, May 9, 2020, 8:36 PM Ben Denhartog notifications@github.com wrote:

I've tested the above macro using a branch name, and it works without specifying a ref on the command line, so the issue here is that I can't set the ref property of git.origin to a SHA-1. Based on the documentation, this appears to be a bug.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/google/copybara/issues/126#issuecomment-626254652, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFKR2H6CVIESFVJTSG5X27LRQXZINANCNFSM4M4WPHUQ .

sudoforge commented 4 years ago

@hsudhof Thanks for the reply. is there a particular reason for that? It seems like an unecessary limitation.

sudoforge commented 4 years ago

Actually, after reading your comment, I think there may be a slight miscommunication. I'm attempting to specify a SHA-1 for a git.origin, not a git.github_pr_origin.

hsudhof commented 4 years ago

Let me come in again.

So your main issue was that PR destination expects the origin to supply a context reference. This is admittedly just for coming up with a meaningful name for the PR branch and can be sidestepped with the flag as stated in the output - there should be a TODO in GitHubPrDestination, will see when we can find the time to revisit this. In addition you can also specify the revision in addition to the ref, e.g. invoke copybara as "copybara " Hardcoding the sha1 in the copy.bara.sky is not really what we had considered as a possible use case, I would prefer to cover such variability with flags or command line params.

I hear your request for a simple "ls" command - the output of INFO is a bit hard to parse. Please file an issue with the use case and ideal output you would have in mind :)

sudoforge commented 4 years ago

Hardcoding the sha1 in the copy.bara.sky is not really what we had considered as a possible use case, I would prefer to cover such variability with flags or command line params.

Admittedly, this exercise is exploring the use of Copybara in what I'm gathering is a non-standard way: as a manager of vendored dependencies. As I understand it, Copybara was born to export code out of Piper into individual Git repositories on e.g. GitHub and Gerrit; not the other way around. I'm exploring its use as a dependency manager as I build out a polyglot-language monorepo.

While there are various build tools that work well in this type of repository (Bazel, Buck, Pants, Plz), the general approach in the open source world is to fetch dependencies and cache them using some mechanism (e.g. the build tool and a remote cache); if you want to vendor them for the various benefits that brings, there is no good generic solution (that I'm aware of) -- you have to build that custom functionality per-language (or really, per-protocol-that-you-want-to-support). This is where I'd like to use Copybara for the inverse of its designed use-case: to read a manifest (copy.bara.sky) defining "import" workflows, and do its thing.

This is where I found the desire to specify the reference inline, as it avoids having to maintain a separate manifest outside of the copy.bara.sky file (e.g. //third_party/golang/MODULES or such) which would require writing some additional parser and then invoking the "download and add process", which could invoke curl + git-add . + git-commit or copybara migrate --init-history ...; the implementation is a non-issue, it's just the matter of having two disjointed processes that I want to avoid.

Am I thinking about this incorrectly? Maybe. I'd welcome your input on the subject, as it's a problem that's been swimming around in my head for a little while, and others' insight is always useful.

hsudhof commented 4 years ago

Let me see about gazelle, this is something that might fit into a module; at the very least we would be open to accepting a contribution for gazelle support.

google / copybara

git.origin -> git.github_pr_destination fails when specifying a SHA-1 `ref` on the origin #126