Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Also track other patches as "pull requests", for easy central visibility as to CI status? #948

Open
TaoK opened this issue Apr 5, 2022 · 8 comments

Comments

@TaoK
Copy link

TaoK commented Apr 5, 2022

I'm not much of a github user, and I haven't understood how GitGitGadget actually "works" yet, so my apologies if this is a stupid question/request:

Would it be possible and would it make sense to also track, in GitGitGadget, the patches that are submitted "natively" to the mailing list as "ownerless" pull requests?

The ability to automatically see build status, and to see and compare different versions of patches without explicitly/manually going through an email-to-branch workflow would, I assume, be a useful thing for at least some users / potential reviewers?

@derrickstolee
Copy link
Contributor

The tricky part is deciding how to pick a starting point to apply the patches. I tried this a few years ago and had difficulty.

If we focus on matching threads that already have a tracking branch in gitster/git, then we could probably do that more easily. The feedback cycle is delayed until that branch is created, though.

@dscho
Copy link
Member

dscho commented Apr 8, 2022

I think we can leave this ticket open for a while, but unless nobody has any splendid idea how to deal with the base commit problem, we will probably have to close the ticket eventually without being able to do anything about it.

@phil-blain
Copy link
Contributor

@TaoK I don't know if you are aware, but there is a Patchwork instance that tracks the patches submitted to the Git mailing list: https://patchwork.kernel.org/project/git/list/. I've used that along with git pw to apply patches from the list locally (granted it does not help for seeing the build status).

These days I mostly use b4 (https://pypi.org/project/b4/) to apply patches locally.

Regarding the base commit, maybe we could track only branches that do have a base-commit trailer in the cover letter ?

@dscho
Copy link
Member

dscho commented May 3, 2022

Regarding the base commit, maybe we could track only branches that do have a base-commit trailer in the cover letter ?

Maybe? But that would be inconsistent: some patches get integrated, some are not. I'm less and less in favor of this: if the contributor does not see fit to publish their work publicly, maybe that is not a problem we should fix on GitGitGadget's side.

@TaoK
Copy link
Author

TaoK commented May 5, 2022

@TaoK I don't know if you are aware, but there is a Patchwork instance

I did not know what patchwork was (I still don't fully get it), and I didn't know there was a public instance that can help here, thank you.

I've tried using git-pw and got stuck at authentication - the project page claims credentials are optional, but a command like git pw series apply 636887 complains that "Authentication information missing". I guess I could register for an account on https://patchwork.kernel.org/register/, but it seems a bit... excessive.

I've tried using b4 but ended up with random environmental issues I haven't understood, or investigated (ImportError: No module named policy).

Without these tools, if I understand correctly, the simplest process to get a patch branch locally is:

  • Identify the patch of interest at https://patchwork.kernel.org/project/git/list/
  • Download the patch or series using the link in the upper-right-hand corner
  • Create/checkout a branch locally, from origin/master normally (?)
  • Run git am <patch-file>
    • If it applied cleanly, yay!
    • Otherwise, hunt in the patch for a "base-commit" trailer in case it was added by GitGitGadget (or anything else?), create a branch from there, and try again...? (or just figure out why you have conflicts, what they are, and hope you understand the patch's intent and context well enough to resolve them?)
  • Once you have a branch with patch (series) applied, push it to your own github fork of GitGitGadget, and that should build automatically...

Regarding the base commit, maybe we could track only branches that do have a base-commit trailer in the cover letter ?

If I'm understanding correctly, that trailer is only added by GitGitGadget... is that right?

@TaoK
Copy link
Author

TaoK commented May 5, 2022

if the contributor does not see fit to publish their work publicly, maybe that is not a problem we should fix on GitGitGadget's side.

I don't understand what you mean here; my intent in starting this thread was to understand how to work with patches submitted by people not using GitGitGadget, for whatever reason (for example, recently, a git-p4 patch series I thought I could help validate).

Clearly the author did intend to publish their work publicly - otherwise they wouldn't have submitted their patch series to the list!

I was eased into publishing work to the git mailing list reasonably-coherently via GitGitGadget - and thank you for that! Insofar as I'm being at all useful it is fully because of GitGitGadge; it would never have occurred to me follow a "raw" patch submission process, and I would have fallen afoul of all the rules had I tried!

My "problem" is that I still haven't understood what a correspondingly reasonable way to consume patches is - hence this thread. @phil-blain has certainly helped me onto what looks like a reasonable path.

@dscho
Copy link
Member

dscho commented May 5, 2022

Regarding the base commit, maybe we could track only branches that do have a base-commit trailer in the cover letter ?

If I'm understanding correctly, that trailer is only added by GitGitGadget... is that right?

It's not specific to GitGitGadget, it's git format-patch's --base=<commit> option that does it, and GitGitGadget uses this option.

There is support for --base=auto, but apparently it isn't turned on by default.

if the contributor does not see fit to publish their work publicly, maybe that is not a problem we should fix on GitGitGadget's side.

I don't understand what you mean here; my intent in starting this thread was to understand how to work with patches submitted by people not using GitGitGadget, for whatever reason (for example, recently, a git-p4 patch series I thought I could help validate).

Right. And the main problem I see there is that they send patches, and often do not even publish the original Git branches, which already means that we lost context. And by "context", I refer to the full revision and commit history, including the actual base commit and most importantly the full set of files/symlinks present in the original commit that reflects precisely what the contributor tested, not just the patches (which might not even apply to the current main branch, or if they do, might interact poorly with independent patches present in the main branch but not in the branch on which the contributor worked locally).

We could try our best to reconstruct said context, but it is not even possible in general. Therefore, we would be limited to a best effort that won't always work.

A fundamental, underlying problem, of course, is that the Git mailing list process requires this lossy way to contribute patches, rather than using a tool based on Git that does not lose the context.

@phil-blain
Copy link
Contributor

@TaoK I'm sorry you're having trouble installing / using b4. It's really a charm to use, and I haven't used git pw since I learned about b4's existence. Usually python3 -m pip install --user b4 should do the trick, if not Konstantin is prompt to help if you write to tools@linux.kernel.org (see https://git.kernel.org/pub/scm/utils/b4/b4.git/tree/README.rst).

I do remember I had to create an account on patchwork so that git pw could authenticate, yes.

Without these tools, if I understand correctly, the simplest process to get a patch branch locally is:

For downloading patches, you can also use the mailing list archive at
lore.kernel.org directly, no need for patchwork. At the bottom of each thread there is
a download link for the whole thread as a gzipped mbox: (e.g. https://lore.kernel.org/git/pull.1227.git.1651324796892.gitgitgadget@gmail.com/T/#t).

Another thing that can help: for topics that the maintainer merged to seen, there is a corresponding topic branch in Junio's fork, prefixed with the initials of the contributor: here are your branches, (mixed with other people sharing your initials).
So you can do something like:

git ls-remote https://github.com/gitster/git | grep tk/
# choose a branch
git fetch  https://github.com/gitster/git  refs/heads/tk/p4-utf8-bom
git checkout FETCH_HEAD
# review, build, test, etc.

This has the advantage that for these topics Junio already chose a base commit if one was not provided by the contributor, so you are guaranteed to not have conflicts.

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

No branches or pull requests

4 participants