vermiculus / magithub

**DEPRECATED - please use Forge instead!** -- Magit-based interfaces to GitHub
GNU General Public License v3.0
579 stars 63 forks source link

Submitting pull requests fails #137

Closed ericdanan closed 6 years ago

ericdanan commented 7 years ago

Hi, I have tried to submit two PRs to two different repos from magithub, but it failed both times with the same error. On the other hand, submitting the PR from the github website worked.

Here is the error for a PR I tried to submit to abo-abo/swiper from magithub (I then successfully submitted it from the website, see https://github.com/abo-abo/swiper/pull/1182):

Unprocessable Entity: "POST", "/repos/abo-abo/swiper/pulls", nil,
"{\"maintainer_can_modify\":true,
  \"title\":\"ensure action and display transformer are called from initial buffer\",
  \"body\":\"This was already implemented for `ivy-call` by commit 50bb6b3. This PR does the same thing for `ivy-occur-press` and `ivy--format`.\",
  \"base\":\"origin/master\",
  \"head\":\"ericdanan/buffer\"}",
((message . "Validation Failed")
 (errors ((resource . "PullRequest")
          (field . "base")
          (code . "invalid"))
         ((resource . "PullRequest")
          (field . "head")
          (code . "invalid")))
 (documentation_url . "https://developer.github.com/v3/pulls/#create-a-pull-request"))

I don't understand what is the problem and would be grateful if have any clue. Of course let me know if I should provide more information.

vermiculus commented 7 years ago

Are you sure you've got the most up-to-date version? I'll admit that PRs aren't something I can fully test (since it's POSTing data) – rather, I've not invested the time to mock the API – but this is similar in effect to #102 which is fixed.

The problem is that your head field is ericdanan/buffer instead of ericdanan:buffer, but this is set correctly at magithub-pull-request-new. If you're not using that function, can you give more details on your workflow?

NicholasTD07 commented 7 years ago

Got a similar error when trying to submit a PR.

I was using the Submit a pull request in magithub-dispatch-popup.

magithub version: 20170901.1234

Unprocessable Entity: "POST", "/repos/some-group/some-repo/pulls", nil,
"{\"maintainer_can_modify\":true,\"title\":\"title for PR\",
\"body\":\"the body of the PR\",\"base\":\"staging\",\"head\":\"NicholasTD07:task/branch\"}",
((message . "Validation Failed") (errors ((resource . "PullRequest") (field . "head")
(code . "invalid")))
(documentation_url . "https://developer.github.com/v3/pulls/#create-a-pull-request"))

My assumption is that the head should not be the value in the posted data.

I checked the code and I ran the following snippet in the same repo.

(let* ((repo        (magithub-repo))
       (upstream    (magit-get-upstream-branch))
       (head        (magit-read-branch "Head"))
       (base        (magit-read-branch "Base branch" upstream))
       (head-remote (magit-get-push-remote head))
       (base-remote (magit-get-push-remote base)))
  (print repo t)
  (print upstream t)
  (print head t)
  (print base t)
  (print head-remote t)
  (print base-remote t)
  )

The head in the output is correct as in it has the value of task/branch.

Let me know how can I help :)

PS.

Great project. I am loving it :)

ericdanan commented 7 years ago

Yes I am using the latest version, and also creating the PR from the magithub dispatch popup.

zakame commented 7 years ago

:+1: also getting this issue, as my workflow is to make PRs from my fork (e.g. zakame/perl6-doc:split-and-rephrase-lines-in-footer) to base (perl6/doc:master). Here's one from my *Messages* buffer about a failed PR creation (slightly edited):

Unprocessable Entity: "POST", "/repos/perl6/doc/pulls", nil,
"{\"maintainer_can_modify\":true,
\"title\":\"Split and rephrase lines in the footer\",
\"body\":\"Blah.\",
\"base\":\"master\",
\"head\":\"split-and-rephrase-lines-in-footer\"}",
((message . "Validation Failed")
 (errors ((resource . "PullRequest") (field . "head") (code . "invalid")))
 (documentation_url . "https://developer.github.com/v3/pulls/#create-a-pull-request"))

It would seem that magithub is failing to resolve the head name correctly.

vermiculus commented 7 years ago

I'll look at this tonight. I'll just have to set up a few dummy repositories 😄

vermiculus commented 7 years ago

Update: I've been having separate troubles with magit tonight (probably related to the performance investigation from a few days ago). I'm probably not going to be able to touch this until the weekend.

anquegi commented 7 years ago

i'm also getting this issue:

Unprocessable Entity: "POST", "/repos/MY_COMPANY/antifraud/pulls", nil, "{\"title\":\"update gem file\",\"body\":\"This is an update of the gem file for using github repos\",\"base\":\"master\",\"head\":\"origin/master\"}", ((message . "Validation Failed") (errors ((resource . "PullRequest") (field . "head") (code . "invalid"))) (documentation_url . "https://developer.github.com/v3/pulls/#create-a-pull-request"))

vermiculus commented 7 years ago

In the pull request buffer, can you give the value of magithub-issue--extra-data? (It's a buffer-local variable, so where you evaluate it is important.) When I evaluate this symbol, I get

((base . "master")
 (head . "develop")
 (kind . pull-request))

which is not what's reflected in the errors you guys are posting ☹️

vermiculus commented 7 years ago

Ohhh, folks -- I think I know what's happening here. When you start writing a new PR, don't select the remote branch (e.g., origin/develop) for your head – select your local tracking branch. As long as the names match up, this should work for now.

I'll look at streamlining this selection process and making it more difficult to make this error. (Really, we should also look to only use heads/bases that exist on the remotes in play.)

Can someone verify that this workaround works for them before I move on to implementing that?

ericdanan commented 7 years ago

Tried to submit a PR to magithub to test this, without success:

Unprocessable Entity: "POST", "/repos/vermiculus/magithub/pulls", nil, "{\"maintainer_can_modify\":true,\"title\":\"Not for merging: test submitting PR\",\"body\":\"This is to test the instructions given in #137.\",\"base\":\"master\",\"head\":\"submitpr\"}", ((message . "Validation Failed") (errors ((resource . "PullRequest") (field . "head") (code . "invalid"))) (documentation_url . "https://developer.github.com/v3/pulls/#create-a-pull-request"))

Evaluating magithub-issue--extra-data from the PR seems correct:

((base . "master") (head . "submitpr") (kind . pull-request))

Thanks

vermiculus commented 7 years ago

To keep this repository clean, we can use https://github.com/vermiculus/my-new-repository for this kind of stuff.

anquegi commented 7 years ago

Hi, I tried but maybe I dd o not do it well:

captura de pantalla 2017-09-12 a las 8 21 28

getting on the messages:

Unprocessable Entity: "POST", "/repos/Telefonica/antifraud/pulls", nil, "{\"maintainer_can_modify\":true,\"title\":\"change gemfile\",\"body\":\"change 11paths private repository to github\",\"base\":\"origin/master\",\"head\":\"master\"}", ((message . "Validation Failed") (errors ((resource . "PullRequest") (field . "base") (code . "invalid"))) (documentation_url . "https://developer.github.com/v3/pulls/#create-a-pull-request"))

and ((base . "origin/master") (head . "master") (kind . pull-request))

vermiculus commented 6 years ago

Alright, folks. Sorry for the long delay in getting to this! There were a lot of unstaged changes on my machine and I needed a good chunk of free time to dedicate to getting it all organized and pushed 😄

The above commit should fix this issue; I'll be merging into master as soon as tests pass.

vermiculus commented 6 years ago

Let me know if you run into any problems with this fix!

ericdanan commented 6 years ago

I updated magithub after commit 043f3a8 and tried to send a PR to vermiculus/my-new-repository still fails for me:

Unprocessable Entity: "POST", "/repos/vermiculus/my-new-repository/pulls", nil, "{\"maintainer_can_modify\":true,\"title\":\"test if submitting PR works\",\"body\":\"After magithub commit 043f3a8f3c5c51b5df6c2415ebc41d588d925ffa.\",\"base\":\"ericdanan:testbranch\",\"head\":\"origin/master\"}", ((message . "Validation Failed") (errors ((resource . "PullRequest") (field . "base") (code . "invalid"))) (documentation_url . "https://developer.github.com/v3/pulls/#create-a-pull-request"))
vermiculus commented 6 years ago

@ericdanan Can you investigate how it's getting origin/master as an available option? Is should just be head:master; I have seen this issue and I thought I fixed it (see the message for 043f3a8f3c5c51b5df6c2415ebc41d588d925ffa).

You should be able to just instrument magithub-pull-request-new-arguments and run that in a scratch buffer; that'll get you the necessary values without needing to create a PR.


Alternatively, you might just be able to eval this for debugging:

(magit-get-upstream-branch
 (magit-read-remote-branch
  "Head"
  (car (magithub-repo-remotes-for-repo
        (magithub-read-repo "Fork's remote (this is you!)")))))
vermiculus commented 6 years ago

@ericdanan Can you give the above a shot? 😄

ericdanan commented 6 years ago

Still not working...

Unprocessable Entity: "POST", "/repos/vermiculus/my-new-repository/pulls", nil, "{\"maintainer_can_modify\":true,\"title\":\"test if submitting PR from magithub works\",\"body\":\"Re magithub issue #137.\",\"base\":\"ericdanan:testbranch\",\"head\":\"origin/master\"}", ((message . "Validation Failed") (errors ((resource . "PullRequest") (field . "base") (code . "invalid"))) (documentation_url . "https://developer.github.com/v3/pulls/#create-a-pull-request"))

What can I do to help investigate the issue?

vermiculus commented 6 years ago

Your head value is still "origin/master"? I simply don't understand how that's possible anymore.

Instrument magithub-pull-request-new-arguments and find out why head is getting the value it's getting.

What other options do you have when choosing the head branch?

vermiculus commented 6 years ago

Wait a second, the validation error says it's the base field that's wrong. (head is also still wrong.)

Did you push testbranch to your fork?

ericdanan commented 6 years ago

Yes testbranch is pushed to ericdanan/my-new-repository. If it matters, here is my git config file:

[core]
    repositoryformatversion = 0
    filemode = true
    bare = false
    logallrefupdates = true
    ignorecase = true
[remote "origin"]
    url = git@github.com:vermiculus/my-new-repository.git
    fetch = +refs/heads/*:refs/remotes/origin/*
[remote "ericdanan"]
    url = git@github.com:ericdanan/my-new-repository.git
    fetch = +refs/heads/*:refs/remotes/ericdanan/*
[remote]
    pushDefault = ericdanan
[branch "master"]
    remote = origin
    merge = refs/heads/master
[branch "testbranch"]
    remote = origin
    merge = refs/heads/master

From my local testbanch branch:

Unprocessable Entity: "POST", "/repos/vermiculus/my-new-repository/pulls", nil, "{\"maintainer_can_modify\":true,\"title\":\"test if submitting PR from magithub works\",\"body\":\"Re magithub issue #137.\",\"base\":\"ericdanan:testbranch\",\"head\":\"master\"}", ((message . "Validation Failed") (errors ((resource . "PullRequest") (field . "base") (code . "invalid"))) (documentation_url . "https://developer.github.com/v3/pulls/#create-a-pull-request"))

So that explains the origin/master instead of master in head. But it still fails. Shouldn't it be base:master, head:ericdanan:testbranch instead of base:ericdanan:testbranch, head:master?

vermiculus commented 6 years ago

Ugh, I feel dumb; I re-ordered the arguments when I pulled the form out into magithub-pull-request-new-arguments.

vermiculus commented 6 years ago

@ericdanan Try the above patch or just switch head/base in the argument listing of magithub-pull-request-new.

ericdanan commented 6 years ago

It works now! You have a dummy PR in vermiculus/my-new-repository that was submitted from magithub.

Thanks!

vermiculus commented 6 years ago

Great! Is it still possible for you to choose origin/master, though? Good software doesn't just work; it makes mistakes impossible.

If it's still possible, I'd like to reopen this issue (or probably rather start a new related one) to investigate why this happens.

ericdanan commented 6 years ago

Yes I can choose origin/master as base branch.

Also I can choose vermiculus/magithub as fork's remote. I don't know if it makes sense to systematically use the pushremote instead of querying the user.

On Tue, Oct 10, 2017 at 2:58 PM, Sean Allred notifications@github.com wrote:

Great! Is it still possible for you to choose origin/master, though? Good software doesn't just work; it makes mistakes impossible.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/vermiculus/magithub/issues/137#issuecomment-335463734, or mute the thread https://github.com/notifications/unsubscribe-auth/ALEs2Rw6jhaYBdbz3e6wV7LJrbIhSIwiks5sq2nlgaJpZM4PMN0i .

vermiculus commented 6 years ago

Created a new issue to investigate.