* odd patch issue @ 2016-03-14 19:46 kevin 2016-03-14 21:20 ` john 0 siblings, 1 reply; 5+ messages in thread From: kevin @ 2016-03-14 19:46 UTC (permalink / raw) Greetings. We are seeing an odd behavior with diffs/patches here. I think it's a bug, but perhaps we are doing something wrong. ;) If you look at: https://fedorapeople.org/cgit/adelton/public_git/CGI-sessions.git/diff/app.cgi?id=additional-attributes you can see just the diff for that one file. However, if you try and get a patch for just that one file: https://fedorapeople.org/cgit/adelton/public_git/CGI-sessions.git/patch/app.cgi?id=additional-attributes You get also any other files affected in that patch. (ie, not just app.cgi). Is this expected? or a bug? This is version 0.12. kevin -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: OpenPGP digital signature URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20160314/2f6b4025/attachment.asc> ^ permalink raw reply [flat|nested] 5+ messages in thread
* odd patch issue 2016-03-14 19:46 odd patch issue kevin @ 2016-03-14 21:20 ` john 2016-03-14 21:44 ` kevin 0 siblings, 1 reply; 5+ messages in thread From: john @ 2016-03-14 21:20 UTC (permalink / raw) On Mon, Mar 14, 2016 at 01:46:29PM -0600, Kevin Fenzi wrote: > We are seeing an odd behavior with diffs/patches here. I think it's a > bug, but perhaps we are doing something wrong. ;) > > If you look at: > > https://fedorapeople.org/cgit/adelton/public_git/CGI-sessions.git/diff/app.cgi?id=additional-attributes > > you can see just the diff for that one file. > However, if you try and get a patch for just that one file: > > https://fedorapeople.org/cgit/adelton/public_git/CGI-sessions.git/patch/app.cgi?id=additional-attributes > > You get also any other files affected in that patch. (ie, not just > app.cgi). Is this expected? or a bug? I think this is expected. It doesn't really make sense for "patch" to use a path since it includes the header and commit message, so it doesn't take a path argument. (Hopefully you created that URL manually and didn't follow a link - if we generate such a link that is a bug.) If you want a diff without all the HTML markup you can use "rawdiff": https://fedorapeople.org/cgit/adelton/public_git/CGI-sessions.git/rawdiff/app.cgi?id=additional-attributes ^ permalink raw reply [flat|nested] 5+ messages in thread
* odd patch issue 2016-03-14 21:20 ` john @ 2016-03-14 21:44 ` kevin 2016-03-14 22:44 ` john 0 siblings, 1 reply; 5+ messages in thread From: kevin @ 2016-03-14 21:44 UTC (permalink / raw) On Mon, 14 Mar 2016 21:20:37 +0000 John Keeping <john at keeping.me.uk> wrote: > On Mon, Mar 14, 2016 at 01:46:29PM -0600, Kevin Fenzi wrote: > > We are seeing an odd behavior with diffs/patches here. I think it's > > a bug, but perhaps we are doing something wrong. ;) > > > > If you look at: > > > > https://fedorapeople.org/cgit/adelton/public_git/CGI-sessions.git/diff/app.cgi?id=additional-attributes > > > > you can see just the diff for that one file. > > However, if you try and get a patch for just that one file: > > > > https://fedorapeople.org/cgit/adelton/public_git/CGI-sessions.git/patch/app.cgi?id=additional-attributes > > > > You get also any other files affected in that patch. (ie, not just > > app.cgi). Is this expected? or a bug? > > I think this is expected. It doesn't really make sense for "patch" to > use a path since it includes the header and commit message, so it > doesn't take a path argument. (Hopefully you created that URL > manually and didn't follow a link - if we generate such a link that > is a bug.) The downstream reporter claims that this worked as they want in the past. ;( https://fedorahosted.org/fedora-infrastructure/ticket/4811 > If you want a diff without all the HTML markup you can use "rawdiff": > > https://fedorapeople.org/cgit/adelton/public_git/CGI-sessions.git/rawdiff/app.cgi?id=additional-attributes Well, I think they want a partial patch thats just the file they list and not the entire patch. (I am not sure why). kevin -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: OpenPGP digital signature URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20160314/c7635dd9/attachment.asc> ^ permalink raw reply [flat|nested] 5+ messages in thread
* odd patch issue 2016-03-14 21:44 ` kevin @ 2016-03-14 22:44 ` john 2016-03-16 18:43 ` kevin 0 siblings, 1 reply; 5+ messages in thread From: john @ 2016-03-14 22:44 UTC (permalink / raw) On Mon, Mar 14, 2016 at 03:44:31PM -0600, Kevin Fenzi wrote: > On Mon, 14 Mar 2016 21:20:37 +0000 > John Keeping <john at keeping.me.uk> wrote: > > > On Mon, Mar 14, 2016 at 01:46:29PM -0600, Kevin Fenzi wrote: > > > We are seeing an odd behavior with diffs/patches here. I think it's > > > a bug, but perhaps we are doing something wrong. ;) > > > > > > If you look at: > > > > > > https://fedorapeople.org/cgit/adelton/public_git/CGI-sessions.git/diff/app.cgi?id=additional-attributes > > > > > > you can see just the diff for that one file. > > > However, if you try and get a patch for just that one file: > > > > > > https://fedorapeople.org/cgit/adelton/public_git/CGI-sessions.git/patch/app.cgi?id=additional-attributes > > > > > > You get also any other files affected in that patch. (ie, not just > > > app.cgi). Is this expected? or a bug? > > > > I think this is expected. It doesn't really make sense for "patch" to > > use a path since it includes the header and commit message, so it > > doesn't take a path argument. (Hopefully you created that URL > > manually and didn't follow a link - if we generate such a link that > > is a bug.) > > The downstream reporter claims that this worked as they want in the > past. ;( > > https://fedorahosted.org/fedora-infrastructure/ticket/4811 Indeed it did, so I think we should do this: -- >8 -- Subject: [PATCH] patch: reapply path limit This was originally applied added in commit eac1b67 (ui-patch: Apply path limit to generated patch, 2010-06-10) but the ability to limit patches to particular paths was lost in commit 455b598 (ui-patch.c: Use log_tree_commit() to generate diffs, 2013-08-20). The new output is slightly different from the original because Git's diff infrastructure doesn't give us a way to insert an annotation immediately after the "---" separator, so the commit has moved below the diff stat. Signed-off-by: John Keeping <john at keeping.me.uk> --- ui-patch.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/ui-patch.c b/ui-patch.c index 4c051e8..e8b5338 100644 --- a/ui-patch.c +++ b/ui-patch.c @@ -18,9 +18,13 @@ void cgit_print_patch(const char *new_rev, const char *old_rev, struct commit *commit; unsigned char new_rev_sha1[20], old_rev_sha1[20]; char rev_range[2 * 40 + 3]; - char *rev_argv[] = { NULL, "--reverse", "--format=email", rev_range }; + const char *rev_argv[] = { NULL, "--reverse", "--format=email", rev_range, "--", prefix }; + int rev_argc = ARRAY_SIZE(rev_argv); char *patchname; + if (!prefix) + rev_argc--; + if (!new_rev) new_rev = ctx.qry.head; @@ -79,7 +83,9 @@ void cgit_print_patch(const char *new_rev, const char *old_rev, rev.max_parents = 1; rev.diffopt.output_format |= DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_PATCH | DIFF_FORMAT_SUMMARY; - setup_revisions(ARRAY_SIZE(rev_argv), (const char **)rev_argv, &rev, + if (prefix) + rev.diffopt.stat_sep = fmt("(limited to '%s')\n\n", prefix); + setup_revisions(ARRAY_SIZE(rev_argv), rev_argv, &rev, NULL); prepare_revision_walk(&rev); -- 2.8.0.rc1.128.gf7e6a6c ^ permalink raw reply [flat|nested] 5+ messages in thread
* odd patch issue 2016-03-14 22:44 ` john @ 2016-03-16 18:43 ` kevin 0 siblings, 0 replies; 5+ messages in thread From: kevin @ 2016-03-16 18:43 UTC (permalink / raw) On Mon, 14 Mar 2016 22:44:50 +0000 John Keeping <john at keeping.me.uk> wrote: > Indeed it did, so I think we should do this: Sorry for the delay here. I finally tested this and indeed it fixes the issue. ;) Thanks! kevin -- > > -- >8 -- > Subject: [PATCH] patch: reapply path limit > > This was originally applied added in commit eac1b67 (ui-patch: Apply > path limit to generated patch, 2010-06-10) but the ability to limit > patches to particular paths was lost in commit 455b598 (ui-patch.c: > Use log_tree_commit() to generate diffs, 2013-08-20). > > The new output is slightly different from the original because Git's > diff infrastructure doesn't give us a way to insert an annotation > immediately after the "---" separator, so the commit has moved below > the diff stat. > > Signed-off-by: John Keeping <john at keeping.me.uk> > --- > ui-patch.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/ui-patch.c b/ui-patch.c > index 4c051e8..e8b5338 100644 > --- a/ui-patch.c > +++ b/ui-patch.c > @@ -18,9 +18,13 @@ void cgit_print_patch(const char *new_rev, const > char *old_rev, struct commit *commit; > unsigned char new_rev_sha1[20], old_rev_sha1[20]; > char rev_range[2 * 40 + 3]; > - char *rev_argv[] = { NULL, "--reverse", "--format=email", > rev_range }; > + const char *rev_argv[] = { NULL, "--reverse", > "--format=email", rev_range, "--", prefix }; > + int rev_argc = ARRAY_SIZE(rev_argv); > char *patchname; > > + if (!prefix) > + rev_argc--; > + > if (!new_rev) > new_rev = ctx.qry.head; > > @@ -79,7 +83,9 @@ void cgit_print_patch(const char *new_rev, const > char *old_rev, rev.max_parents = 1; > rev.diffopt.output_format |= DIFF_FORMAT_DIFFSTAT | > DIFF_FORMAT_PATCH | DIFF_FORMAT_SUMMARY; > - setup_revisions(ARRAY_SIZE(rev_argv), (const char > **)rev_argv, &rev, > + if (prefix) > + rev.diffopt.stat_sep = fmt("(limited to '%s')\n\n", > prefix); > + setup_revisions(ARRAY_SIZE(rev_argv), rev_argv, &rev, > NULL); > prepare_revision_walk(&rev); > -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: OpenPGP digital signature URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20160316/144ca2c0/attachment.asc> ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-03-16 18:43 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-03-14 19:46 odd patch issue kevin 2016-03-14 21:20 ` john 2016-03-14 21:44 ` kevin 2016-03-14 22:44 ` john 2016-03-16 18:43 ` kevin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).