git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Alejandro Sanchez <alex@schedmd.com>
Cc: git@vger.kernel.org
Subject: Re: git am / patch utility not applying a .patch to the correct function
Date: Thu, 29 Apr 2021 15:45:56 -0400	[thread overview]
Message-ID: <YIsM9DDAoqywKmnK@coredump.intra.peff.net> (raw)
In-Reply-To: <CAF-Z3RqEz9mnnosew+TfNkV2ysaTkGj_ZXpD-4NWO7th9v_-CA@mail.gmail.com>

On Thu, Apr 29, 2021 at 04:25:42PM +0200, Alejandro Sanchez wrote:

> When I attempted to git am a patch clearly targeted for
> _eval_nodes_dfly(), the result shown by git show resulted in the
> modified code in _eval_nodes_topo() instead. A colleague also reported
> this is also happening with the 'patch' utility without git, we're not
> sure if they both share any common logic or library behind the scenes.

They don't share code, but this is an inherent issue with patches. If
the file you're applying the patch to is not at the exact same state as
the original (say, some other content has been introduced which moves
the patched lines around), then the tools will use the context lines in
the patch to try to find the right spot.

Usually this does what you want, but it can occasionally find the wrong
spot (e.g., if the lines in the context are repeated, and especially if
the "right" spot was changed so that the context doesn't match).

> I've attached a sequence of commands showing the contents of the patch
> and the result after applying.

Your patch here is corrupted:

> diff --git a/src/plugins/select/cons_tres/job_test.c b/src/plugins/select/cons_tres/job_test.c
> index 2d25345945..ff125fafa3 100644
> --- a/src/plugins/select/cons_tres/job_test.c
> +++ b/src/plugins/select/cons_tres/job_test.c
> @@ -2045,7 +2045,8 @@ static int _eval_nodes_dfly(job_record_t *job_ptr,
>  	rc = SLURM_ERROR;
>  fini:
> -	if (job_ptr->req_switch > 0 && rc == SLURM_SUCCESS) {
> +	if (job_ptr->req_switch > 0 && switch_node_bitmap &&
> +	    rc == SLURM_SUCCESS) {
>  		int leaf_switch_count = 0;
>  		/* Count up leaf switches. */

It claims 7 lines in the pre-image, but there are only 5 (context lines
+ the "-" lien). And likewise, 2 lines are missing from the post-image.

I'm guessing this has to do with cutting-and-pasting into the mail,
because your "git show" output is likewise corrupted. The correct patch
is:

diff --git a/src/plugins/select/cons_tres/job_test.c b/src/plugins/select/cons_tres/job_test.c
index 2d25345945..ff125fafa3 100644
--- a/src/plugins/select/cons_tres/job_test.c
+++ b/src/plugins/select/cons_tres/job_test.c
@@ -2045,7 +2045,8 @@ static int _eval_nodes_dfly(job_record_t *job_ptr,
 	rc = SLURM_ERROR;
 
 fini:
-	if (job_ptr->req_switch > 0 && rc == SLURM_SUCCESS) {
+	if (job_ptr->req_switch > 0 && switch_node_bitmap &&
+	    rc == SLURM_SUCCESS) {
 		int leaf_switch_count = 0;
 
 		/* Count up leaf switches. */

Starting at the state of 63e94c2ccbb62aea84cfb0b808761de2bb64e74c^,
which you mentioned, the context doesn't match line 2045 (which is in
_eval_nodes_dfly):

  $ sed -n '2045,+7p' src/plugins/select/cons_tres/job_test.c
  	rc = SLURM_ERROR;
  
  fini:
  	if (job_ptr->req_switch > 0 && rc == SLURM_SUCCESS) {
  		/* req_switch == 1 here; enforced at the top of the function. */
  		leaf_switch_count = 0;
  
  		/* count up leaf switches */

There's an extra comment at the top of the block, and the case differs
in the lower comment. But the context _does_ match the later block of
code in _eval_nodes_topo():

  $ sed -ne '2626,+7p' src/plugins/select/cons_tres/job_test.c
  	rc = SLURM_ERROR;
  
  fini:
  	if (job_ptr->req_switch > 0 && switch_node_bitmap &&
  	    rc == SLURM_SUCCESS) {
  		int leaf_switch_count = 0;
  
  		/* Count up leaf switches. */

And hence that's where both git and patch will apply it.

One other confusion may be that the hunk header says
"_eval_nodes_dfly()" in it. This is purely informational for a human
reader, and not taken into account by the patch application code (which
does not even understand things like functions in the first place).

-Peff

      reply	other threads:[~2021-04-29 19:46 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-29 14:25 Alejandro Sanchez
2021-04-29 19:45 ` Jeff King [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YIsM9DDAoqywKmnK@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=alex@schedmd.com \
    --cc=git@vger.kernel.org \
    --subject='Re: git am / patch utility not applying a .patch to the correct function' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).