"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
Re: check file status before applying patches (second try)
To:
Stefan Sperling <stsp@stsp.name>
Cc:
gameoftrees@openbsd.org
Date:
Sun, 13 Mar 2022 12:53:31 +0100

Download raw body.

Thread
  • Omar Polo:

    check file status before applying patches (second try)

  • Stefan Sperling <stsp@stsp.name> wrote:
    > >  	if (p->old != NULL && p->new == NULL) {
    > >  		/*
    > >  		 * special case: delete a file.  don't try to match
    > >  		 * the lines but just schedule the removal.
    > >  		 */
    > 
    > In the long term, this code should try to match the lines.
    > If lines don't match then we are deleting "with fuzz", which
    > might not be what the user wants. It should at least result in a
    > warning or fuzz factor or similar appearing in progress output.
    
    I was posticipating that, but at least for the deletion case the fuzzy
    is easy to implement so why posticipate at all?  here's a draft of how
    it would work, with a test case even :)
    
    (i'm not sure if/how to document that.  could we keep it a secret until
    i manage to implement a real fuzzy for all cases?)
    
    -----------------------------------------------
    commit 49cb05898eb3b998a3eda6b4b64f8f95b114a01b (patchcfs)
    from: Omar Polo <op@omarpolo.com>
    date: Sun Mar 13 11:47:50 2022 UTC
     
     got patch: use fuzzying when deleting files
     
     The current behaviour wrt file deletions is too simple: the file gets
     scheduled for deletion regardless of its content.
     
     This improves the situation by using a smaller fuzzying factor and still
     runnig the patch on the file.  Up to two lines of mismatching are
     allowed, more will cause the "patch don't apply" error.
     
     (note that fuzzying is still not used in the other cases)
     
    diff 9eec00bfb3e7e3a68db41095f894e1f33a0d4727 fd86da681c2acb5f1c1eef6f656e2661de541f9d
    blob - a8c35b3dbc60b36d31157aa8c3a0e6fb6d789bb3
    blob + b8488c42b2d04ef8d98dc9a5a5eba07739d80def
    --- lib/patch.c
    +++ lib/patch.c
    @@ -26,6 +26,7 @@
     #include <sys/types.h>
     #include <sys/queue.h>
     #include <sys/socket.h>
    +#include <sys/stat.h>
     #include <sys/uio.h>
     
     #include <errno.h>
    @@ -323,7 +324,7 @@ locate_hunk(FILE *orig, struct got_patch_hunk *h, off_
     }
     
     static const struct got_error *
    -test_hunk(FILE *orig, struct got_patch_hunk *h)
    +test_hunk(FILE *orig, struct got_patch_hunk *h, int fuzzy)
     {
     	const struct got_error *err = NULL;
     	char *line = NULL;
    @@ -345,7 +346,7 @@ test_hunk(FILE *orig, struct got_patch_hunk *h)
     					    GOT_ERR_PATCH_DONT_APPLY);
     				goto done;
     			}
    -			if (strcmp(h->lines[i]+1, line)) {
    +			if (strcmp(h->lines[i]+1, line) && --fuzzy < 0) {
     				err = got_error(GOT_ERR_PATCH_DONT_APPLY);
     				goto done;
     			}
    @@ -382,7 +383,7 @@ apply_hunk(FILE *tmp, struct got_patch_hunk *h, long *
     }
     
     static const struct got_error *
    -patch_file(struct got_patch *p, const char *path, FILE *tmp)
    +patch_file(struct got_patch *p, const char *path, FILE *tmp, int fuzzy)
     {
     	const struct got_error *err = NULL;
     	struct got_patch_hunk *h;
    @@ -424,7 +425,7 @@ patch_file(struct got_patch *p, const char *path, FILE
     			goto done;
     		copypos = pos;
     
    -		err = test_hunk(orig, h);
    +		err = test_hunk(orig, h, fuzzy);
     		if (err != NULL && err->code == GOT_ERR_PATCH_DONT_APPLY) {
     			/*
     			 * try to apply the hunk again starting the search
    @@ -456,7 +457,15 @@ patch_file(struct got_patch *p, const char *path, FILE
     		}
     	}
     
    -	if (!feof(orig))
    +	
    +	if (p->new == NULL) {
    +		struct stat sb;
    +
    +		if (fstat(fileno(orig), &sb) == -1)
    +			err = got_error_from_errno("fstat");
    +		else if (sb.st_size != copypos)
    +			err = got_error(GOT_ERR_PATCH_DONT_APPLY);
    +	} else if (!feof(orig))
     		err = copy(tmp, orig, copypos, -1);
     
     done:
    @@ -559,7 +568,7 @@ apply_patch(struct got_worktree *worktree, struct got_
     {
     	const struct got_error *err = NULL;
     	struct got_pathlist_head oldpaths, newpaths;
    -	int file_renamed = 0;
    +	int file_renamed = 0, fuzzy = 0;
     	char *oldpath = NULL, *newpath = NULL;
     	char *tmppath = NULL, *template = NULL;
     	FILE *tmp = NULL;
    @@ -585,16 +594,6 @@ apply_patch(struct got_worktree *worktree, struct got_
     	if (err)
     		goto done;
     
    -	if (p->old != NULL && p->new == NULL) {
    -		/*
    -		 * special case: delete a file.  don't try to match
    -		 * the lines but just schedule the removal.
    -		 */
    -		err = got_worktree_schedule_delete(worktree, &oldpaths,
    -		    0, NULL, delete_cb, delete_arg, repo, 0, 0);
    -		goto done;
    -	}
    -
     	if (asprintf(&template, "%s/got-patch",
     	    got_worktree_get_root_path(worktree)) == -1) {
     		err = got_error_from_errno(template);
    @@ -604,10 +603,18 @@ apply_patch(struct got_worktree *worktree, struct got_
     	err = got_opentemp_named(&tmppath, &tmp, template);
     	if (err)
     		goto done;
    -	err = patch_file(p, oldpath, tmp);
    +	if (p->old != NULL && p->new == NULL)
    +		fuzzy = 2;
    +	err = patch_file(p, oldpath, tmp, fuzzy);
     	if (err)
     		goto done;
     
    +	if (p->old != NULL && p->new == NULL) {
    +		err = got_worktree_schedule_delete(worktree, &oldpaths,
    +		    0, NULL, delete_cb, delete_arg, repo, 0, 0);
    +		goto done;
    +	}
    +
     	if (rename(tmppath, newpath) == -1) {
     		err = got_error_from_errno3("rename", tmppath, newpath);
     		goto done;
    blob - 36c9ef55d45f97cd15c3f787c35d5a7623def72a
    blob + 9a300a816a79b7eacef07263e7bd7cbce706ae25
    --- regress/cmdline/patch.sh
    +++ regress/cmdline/patch.sh
    @@ -439,6 +439,44 @@ EOF
     		return 1
     	fi
     
    +	# try to delete a file with a patch that doesn't match
    +	jot 100 > $testroot/wt/numbers
    +	(cd $testroot/wt && got add numbers && got commit -m 'add numbers') \
    +		>/dev/null
    +	ret=$?
    +	if [ $ret -ne 0 ]; then
    +		test_done $testroot $ret
    +		return 1
    +	fi
    +
    +	cat <<EOF > $testroot/wt/patch
    +--- numbers
    ++++ /dev/null
    +@@ -1,9 +0,0 @@
    +-1
    +-2
    +-3
    +-4
    +-5
    +-6
    +-7
    +-8
    +-9
    +EOF
    +
    +	(cd $testroot/wt && got patch patch) > /dev/null 2> $testroot/stderr
    +	ret=$?
    +	if [ $ret -eq 0 ]; then # should fail
    +		test_done $testroot 1
    +		return 1
    +	fi
    +
    +	echo "got: patch doesn't apply" > $testroot/stderr.expected
    +	cmp -s $testroot/stderr.expected $testroot/stderr
    +	ret=$?
    +	if [ $ret -ne 0 ]; then
    +		diff -u $testroot/stderr.expected $testroot/stderr
    +	fi
     	test_done $testroot $ret
     }
     
    
    
    
    
  • Omar Polo:

    check file status before applying patches (second try)