git bug

git bug

[moti sd]

[-- Attachment #1.1: Type: text/plain, Size: 1985 bytes --]

Dear Git Team,

I am writing to report a potential bug encountered while using the "git
stash" command. The issue was observed during a troubleshooting. Please
find the details below:

*Problem Description:*

The "git stash" command is not providing any feedback or indication of
execution, even though modifications are present and the stash operation is
attempted.
*Steps to Reproduce:*

Make modifications to a file.
Execute "git stash" to stash changes.
*Expected Behavior:*

The "git stash" command should provide feedback on the stash operation,
confirming whether it was successful or if there were any errors.
*Additional Information:*
The issue was initially observed when executing "git commit -a --amend" and
pressing Ctrl+Z to exit. Subsequent attempts to use "git stash" resulted in
an error.
Testing with a provided ZIP file containing a problematic repository
confirmed the issue's recurrence.
*Investigation and Findings:*

Upon investigation, it was discovered that the use of Ctrl+Z after "git
commit -a --amend" might be the root cause.
Deleting a lock file left behind by the incomplete git command resolved the
issue, indicating a potential bug in how "git stash" handles incomplete
commands.
*Recommendation:*

The "git stash" command should provide meaningful feedback or error
messages to users, even in cases where the operation is incomplete or
encounters issues.
*Steps to Reproduce the Bug:*

Execute "git commit -a --amend."
Press Ctrl+Z to exit the command.
Attempt "git stash."
*Attachment:*
A ZIP file containing a problematic repository is attached for your
reference.
*Impact:*

The bug affects users' ability to use the "git stash" command effectively,
leading to potential confusion and hindering workflow.
Resolution:


I appreciate your attention to this matter, and I believe addressing this
bug will enhance the overall user experience with Git. If further
information or testing is required, please feel free to reach out.


Best regards,
Moti

[-- Attachment #1.2: Type: text/html, Size: 2332 bytes --]

[-- Attachment #2: git-bugreport-2024-02-05-0015.txt --]
[-- Type: text/plain, Size: 1262 bytes --]

Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.

What did you do before the bug happened? (Steps to reproduce your issue)
I executed "git commit -a --amend," where I pressed Ctrl+Z to exit. Later, when trying again, an error occurred.

What did you expect to happen? (Expected behavior)
I intended to execute the "git stash" command, but it is not functioning as expected. I wish to receive an error message to better understand the issue and address it accordingly. The command is silent, whereas during a commit, error messages are displayed, providing helpful feedback for resolution.
What happened instead? (Actual behavior)
Due to .git/lock file, git stash does not behave normally, and the error message does not show up

Please review the rest of the bug report below.
You can delete any lines you don't wish to share.


[System Info]
git version:
git version 2.38.1.windows.1
cpu: x86_64
built from commit: b85c8f604d375d4d773a36842964e8a7ec056aae
sizeof-long: 4
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon
uname: Windows 10.0 22631 
compiler info: gnuc: 12.2
libc info: no libc information available
$SHELL (typically, interactive shell): <unset>


[Enabled Hooks]

[-- Attachment #3: git-diagnostics-2024-02-05-0015.zip --]
[-- Type: application/x-zip-compressed, Size: 844 bytes --]

[-- Attachment #4: 02-stash-amend-wd.no-quiere-stashear.zip --]
[-- Type: application/x-zip-compressed, Size: 28391 bytes --]

Re: git bug

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 1748 bytes --]

On Mon, Feb 05, 2024 at 12:42:03AM +0100, moti sd wrote:
> Dear Git Team,
> 
> I am writing to report a potential bug encountered while using the "git
> stash" command. The issue was observed during a troubleshooting. Please
> find the details below:
> 
> *Problem Description:*
> 
> The "git stash" command is not providing any feedback or indication of
> execution, even though modifications are present and the stash operation is
> attempted.
> *Steps to Reproduce:*
> 
> Make modifications to a file.
> Execute "git stash" to stash changes.
> *Expected Behavior:*
> 
> The "git stash" command should provide feedback on the stash operation,
> confirming whether it was successful or if there were any errors.
> *Additional Information:*
> The issue was initially observed when executing "git commit -a --amend" and
> pressing Ctrl+Z to exit. Subsequent attempts to use "git stash" resulted in
> an error.
> Testing with a provided ZIP file containing a problematic repository
> confirmed the issue's recurrence.
> *Investigation and Findings:*
> 
> Upon investigation, it was discovered that the use of Ctrl+Z after "git
> commit -a --amend" might be the root cause.
> Deleting a lock file left behind by the incomplete git command resolved the
> issue, indicating a potential bug in how "git stash" handles incomplete
> commands.
> *Recommendation:*

This is indeed it. The problem is that the index will be locked while
git-commit(1) is active, and thus git-stash(1) cannot write to it at the
same time. So this is working as designed. What's not though is the fact
that we don't print an error message.

I'll send a patch in reply to this message to fix this bug. Thanks for
your report!

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH] builtin/stash: report failure to write to index

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 3630 bytes --]

The git-stash(1) command needs to write to the index for many of its
operations. When the index is locked by a concurrent writer it will thus
fail to operate, which is expected. What is not expected though is that
we do not print any error message at all in this case. The user can thus
easily miss the fact that the command didn't do what they expected it to
do and would be left wondering why that is.

Fix this bug and report failures to write to the index. Add tests for
the subcommands which hit the respective code paths.

Note that the chosen error message ("Cannot write to the index") does
not match our guidelines as it starts with a capitalized letter. This is
intentional though and matches the style of all the other messages used
in git-stash(1).

Reported-by: moti sd <motisd8@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/stash.c  |  6 +++---
 t/t3903-stash.sh | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/builtin/stash.c b/builtin/stash.c
index b2813c614c..9df072b459 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -537,7 +537,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
 	repo_read_index_preload(the_repository, NULL, 0);
 	if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
 					 NULL, NULL, NULL))
-		return -1;
+		return error(_("Cannot write to the index"));
 
 	if (write_index_as_tree(&c_tree, &the_index, get_index_file(), 0,
 				NULL))
@@ -1364,7 +1364,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
 	repo_read_index_preload(the_repository, NULL, 0);
 	if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
 					 NULL, NULL, NULL) < 0) {
-		ret = -1;
+		ret = error(_("Cannot write to the index"));
 		goto done;
 	}
 
@@ -1555,7 +1555,7 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
 
 	if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
 					 NULL, NULL, NULL)) {
-		ret = -1;
+		ret = error(_("Cannot write to the index"));
 		goto done;
 	}
 
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 3319240515..770881e537 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -1516,4 +1516,56 @@ test_expect_success 'restore untracked files even when we hit conflicts' '
 	)
 '
 
+test_expect_success 'stash create reports a locked index' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit A A.file &&
+		echo change >A.file &&
+		touch .git/index.lock &&
+
+		cat >expect <<-EOF &&
+		error: Cannot write to the index
+		EOF
+		test_must_fail git stash create 2>err &&
+		test_cmp expect err
+	)
+'
+
+test_expect_success 'stash push reports a locked index' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit A A.file &&
+		echo change >A.file &&
+		touch .git/index.lock &&
+
+		cat >expect <<-EOF &&
+		error: Cannot write to the index
+		EOF
+		test_must_fail git stash push 2>err &&
+		test_cmp expect err
+	)
+'
+
+test_expect_success 'stash apply reports a locked index' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit A A.file &&
+		echo change >A.file &&
+		git stash push &&
+		touch .git/index.lock &&
+
+		cat >expect <<-EOF &&
+		error: Cannot write to the index
+		EOF
+		test_must_fail git stash apply 2>err &&
+		test_cmp expect err
+	)
+'
+
 test_done
-- 
2.43.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] builtin/stash: report failure to write to index

[Rubén Justo]

On 05-feb-2024 08:01:04, Patrick Steinhardt wrote:
> The git-stash(1) command needs to write to the index for many of its
> operations. When the index is locked by a concurrent writer it will thus
> fail to operate, which is expected. What is not expected though is that
> we do not print any error message at all in this case. The user can thus
> easily miss the fact that the command didn't do what they expected it to
> do and would be left wondering why that is.
> 
> Fix this bug and report failures to write to the index. Add tests for
> the subcommands which hit the respective code paths.
> 
> Note that the chosen error message ("Cannot write to the index") does
> not match our guidelines as it starts with a capitalized letter. This is
> intentional though and matches the style of all the other messages used
> in git-stash(1).

Your message made me curious, so I ran:

   $ git grep -E '(die|error)\(' builtin/stash.c
   
   builtin/stash.c:168:               die(_("'%s' is not a stash-like commit"), revision);
   builtin/stash.c:214:               return error(_("%s is not a valid reference"), revision);
   builtin/stash.c:261:               return error(_("git stash clear with arguments is "
   builtin/stash.c:303:               return error(_("unable to write new index file"));
   builtin/stash.c:487:                                       die("Failed to move %s to %s\n",
   builtin/stash.c:523:               die(_("Unable to write index."));
   builtin/stash.c:544:               return error(_("cannot apply a stash in the middle of a merge"));
   builtin/stash.c:555:                               return error(_("could not generate diff %s^!."),
   builtin/stash.c:562:                               return error(_("conflicts in index. "
   builtin/stash.c:569:                               return error(_("could not save index tree"));
   builtin/stash.c:610:               ret = error(_("could not write index"));
   builtin/stash.c:630:               ret = error(_("could not restore untracked files from stash"));
   builtin/stash.c:702:               return error(_("%s: Could not drop stash entry"),
   builtin/stash.c:721:               return error(_("'%s' is not a stash reference"), info->revision.buf);
   builtin/stash.c:872:                       die(_("failed to parse tree"));
   builtin/stash.c:883:               die(_("failed to unpack trees"));
   builtin/stash.c:1547:              if (report_path_error(ps_matched, ps)) {
   builtin/stash.c:1763:                      die("subcommand wasn't specified; 'push' can't be assumed due to unexpected token '%s'",
   builtin/stash.c:1773:                      die(_("options '%s' and '%s' cannot be used together"), "--pathspec-from-file", "--patch");
   builtin/stash.c:1776:                      die(_("options '%s' and '%s' cannot be used together"), "--pathspec-from-file", "--staged");
   builtin/stash.c:1779:                      die(_("'%s' and pathspec arguments cannot be used together"), "--pathspec-from-file");
   builtin/stash.c:1785:              die(_("the option '%s' requires '%s'"), "--pathspec-file-nul", "--pathspec-from-file");

Certainly, there are some error messages in builtin/stash.c that do not
follow the notes in Documentation/CodingGuideLines, but it does not seem
to be "the style of all other messages" in git-stash.

However, your message is clear ...  What error messages are you
considering?

> 
> Reported-by: moti sd <motisd8@gmail.com>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/stash.c  |  6 +++---
>  t/t3903-stash.sh | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 55 insertions(+), 3 deletions(-)
> 
> diff --git a/builtin/stash.c b/builtin/stash.c
> index b2813c614c..9df072b459 100644
> --- a/builtin/stash.c
> +++ b/builtin/stash.c
> @@ -537,7 +537,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
>  	repo_read_index_preload(the_repository, NULL, 0);
>  	if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
>  					 NULL, NULL, NULL))
> -		return -1;
> +		return error(_("Cannot write to the index"));
>  
>  	if (write_index_as_tree(&c_tree, &the_index, get_index_file(), 0,
>  				NULL))
> @@ -1364,7 +1364,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
>  	repo_read_index_preload(the_repository, NULL, 0);
>  	if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
>  					 NULL, NULL, NULL) < 0) {
> -		ret = -1;
> +		ret = error(_("Cannot write to the index"));
>  		goto done;
>  	}
>  
> @@ -1555,7 +1555,7 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
>  
>  	if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
>  					 NULL, NULL, NULL)) {
> -		ret = -1;
> +		ret = error(_("Cannot write to the index"));
>  		goto done;
>  	}
>  
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index 3319240515..770881e537 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -1516,4 +1516,56 @@ test_expect_success 'restore untracked files even when we hit conflicts' '
>  	)
>  '
>  
> +test_expect_success 'stash create reports a locked index' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit A A.file &&
> +		echo change >A.file &&
> +		touch .git/index.lock &&
> +
> +		cat >expect <<-EOF &&
> +		error: Cannot write to the index
> +		EOF
> +		test_must_fail git stash create 2>err &&
> +		test_cmp expect err
> +	)
> +'
> +
> +test_expect_success 'stash push reports a locked index' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit A A.file &&
> +		echo change >A.file &&
> +		touch .git/index.lock &&
> +
> +		cat >expect <<-EOF &&
> +		error: Cannot write to the index
> +		EOF
> +		test_must_fail git stash push 2>err &&
> +		test_cmp expect err
> +	)
> +'
> +
> +test_expect_success 'stash apply reports a locked index' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit A A.file &&
> +		echo change >A.file &&
> +		git stash push &&
> +		touch .git/index.lock &&
> +
> +		cat >expect <<-EOF &&
> +		error: Cannot write to the index
> +		EOF
> +		test_must_fail git stash apply 2>err &&
> +		test_cmp expect err
> +	)
> +'
> +
>  test_done
> -- 
> 2.43.GIT
> 

Re: [PATCH] builtin/stash: report failure to write to index

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> The git-stash(1) command needs to write to the index for many of its
> operations. When the index is locked by a concurrent writer it will thus
> fail to operate, which is expected. What is not expected though is that
> we do not print any error message at all in this case. The user can thus
> easily miss the fact that the command didn't do what they expected it to
> do and would be left wondering why that is.

Hopefully, they know they notice the exit status of the command, or
do we throw the error away and exit(0) from the program?

In any case, telling the users what did (and did not) happen is a
good idea.

> Fix this bug and report failures to write to the index. Add tests for
> the subcommands which hit the respective code paths.
>
> Note that the chosen error message ("Cannot write to the index") does
> not match our guidelines as it starts with a capitalized letter. This is
> intentional though and matches the style of all the other messages used
> in git-stash(1).

Style may be OK, but I wonder if they should say different things,
to hint what failed.  For example:

> @@ -537,7 +537,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
>  	repo_read_index_preload(the_repository, NULL, 0);
>  	if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
>  					 NULL, NULL, NULL))
> -		return -1;
> +		return error(_("Cannot write to the index"));
>
>  	if (write_index_as_tree(&c_tree, &the_index, get_index_file(), 0,
>  				NULL))

This failure and message comes before anything interesting happens.
We attempted to refresh the current index and failed to write out
the result, meaning that whatever index we had on disk did not get
overwritten.  Is this new message enough to tell the user that we
didn't touch the working tree or the index, which would happen if
even some part of "stash apply" happened?  Or is it obvious that we
did not do anything?

> @@ -1364,7 +1364,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
>  	repo_read_index_preload(the_repository, NULL, 0);
>  	if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
>  					 NULL, NULL, NULL) < 0) {
> -		ret = -1;
> +		ret = error(_("Cannot write to the index"));
>  		goto done;
>  	}

Ditto.

> @@ -1555,7 +1555,7 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
>  
>  	if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
>  					 NULL, NULL, NULL)) {
> -		ret = -1;
> +		ret = error(_("Cannot write to the index"));
>  		goto done;
>  	}
>  

Ditto.

Thanks.

Re: [PATCH] builtin/stash: report failure to write to index

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 9056 bytes --]

On Mon, Feb 05, 2024 at 11:22:01PM +0100, Rubén Justo wrote:
> On 05-feb-2024 08:01:04, Patrick Steinhardt wrote:
> > The git-stash(1) command needs to write to the index for many of its
> > operations. When the index is locked by a concurrent writer it will thus
> > fail to operate, which is expected. What is not expected though is that
> > we do not print any error message at all in this case. The user can thus
> > easily miss the fact that the command didn't do what they expected it to
> > do and would be left wondering why that is.
> > 
> > Fix this bug and report failures to write to the index. Add tests for
> > the subcommands which hit the respective code paths.
> > 
> > Note that the chosen error message ("Cannot write to the index") does
> > not match our guidelines as it starts with a capitalized letter. This is
> > intentional though and matches the style of all the other messages used
> > in git-stash(1).
> 
> Your message made me curious, so I ran:
> 
>    $ git grep -E '(die|error)\(' builtin/stash.c
>    
>    builtin/stash.c:168:               die(_("'%s' is not a stash-like commit"), revision);
>    builtin/stash.c:214:               return error(_("%s is not a valid reference"), revision);
>    builtin/stash.c:261:               return error(_("git stash clear with arguments is "
>    builtin/stash.c:303:               return error(_("unable to write new index file"));
>    builtin/stash.c:487:                                       die("Failed to move %s to %s\n",
>    builtin/stash.c:523:               die(_("Unable to write index."));
>    builtin/stash.c:544:               return error(_("cannot apply a stash in the middle of a merge"));
>    builtin/stash.c:555:                               return error(_("could not generate diff %s^!."),
>    builtin/stash.c:562:                               return error(_("conflicts in index. "
>    builtin/stash.c:569:                               return error(_("could not save index tree"));
>    builtin/stash.c:610:               ret = error(_("could not write index"));
>    builtin/stash.c:630:               ret = error(_("could not restore untracked files from stash"));
>    builtin/stash.c:702:               return error(_("%s: Could not drop stash entry"),
>    builtin/stash.c:721:               return error(_("'%s' is not a stash reference"), info->revision.buf);
>    builtin/stash.c:872:                       die(_("failed to parse tree"));
>    builtin/stash.c:883:               die(_("failed to unpack trees"));
>    builtin/stash.c:1547:              if (report_path_error(ps_matched, ps)) {
>    builtin/stash.c:1763:                      die("subcommand wasn't specified; 'push' can't be assumed due to unexpected token '%s'",
>    builtin/stash.c:1773:                      die(_("options '%s' and '%s' cannot be used together"), "--pathspec-from-file", "--patch");
>    builtin/stash.c:1776:                      die(_("options '%s' and '%s' cannot be used together"), "--pathspec-from-file", "--staged");
>    builtin/stash.c:1779:                      die(_("'%s' and pathspec arguments cannot be used together"), "--pathspec-from-file");
>    builtin/stash.c:1785:              die(_("the option '%s' requires '%s'"), "--pathspec-file-nul", "--pathspec-from-file");
> 
> Certainly, there are some error messages in builtin/stash.c that do not
> follow the notes in Documentation/CodingGuideLines, but it does not seem
> to be "the style of all other messages" in git-stash.
> 
> However, your message is clear ...  What error messages are you
> considering?

That's because many of the code paths don't use either `error()` or
`die()`, but use `fprintf_ln()` instead:

$ git grep 'fprintf_ln(stderr' -- builtin/stash.c
builtin/stash.c:		fprintf_ln(stderr, _("Too many revisions specified:%s"),
builtin/stash.c:			fprintf_ln(stderr, _("No stash entries found."));
builtin/stash.c:			fprintf_ln(stderr, _("Index was not unstashed."));
builtin/stash.c:		fprintf_ln(stderr, _("No branch name specified"));
builtin/stash.c:			fprintf_ln(stderr, _("Cannot update %s with %s"),
builtin/stash.c:			fprintf_ln(stderr, _("\"git stash store\" requires one "
builtin/stash.c:			fprintf_ln(stderr, _("Cannot update %s with %s"),
builtin/stash.c:			fprintf_ln(stderr, _("No staged changes"));
builtin/stash.c:			fprintf_ln(stderr, _("No changes selected"));
builtin/stash.c:			fprintf_ln(stderr, _("You do not have "
builtin/stash.c:			fprintf_ln(stderr, _("Cannot save the current "
builtin/stash.c:				fprintf_ln(stderr, _("Cannot save "
builtin/stash.c:				fprintf_ln(stderr, _("Cannot save the current "
builtin/stash.c:				fprintf_ln(stderr, _("Cannot save the current "
builtin/stash.c:				fprintf_ln(stderr, _("Cannot save the current "
builtin/stash.c:			fprintf_ln(stderr, _("Cannot record "
builtin/stash.c:		fprintf_ln(stderr, _("Can't use --patch and --include-untracked"
builtin/stash.c:		fprintf_ln(stderr, _("Can't use --staged and --include-untracked"
builtin/stash.c:			fprintf_ln(stderr, _("Did you forget to 'git add'?"));
builtin/stash.c:			fprintf_ln(stderr, _("Cannot initialize stash"));
builtin/stash.c:			fprintf_ln(stderr, _("Cannot save the current status"));
builtin/stash.c:				fprintf_ln(stderr, _("Cannot remove "

Overall, "builtin/stash.c" is a wild mixture of error styles. Some are
likely user-facing, where it might be a good idea to use `fprinf_ln()`
instead of `error()`. But some of them are closer to what we do in this
patch and likely should be converted to use `error()`, too.

I dunno, I find this to be confusing. But I spotted that there's an
instance already where we say `error("cannot write index")`, so I'll
just use that.

Patrick

> > 
> > Reported-by: moti sd <motisd8@gmail.com>
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> >  builtin/stash.c  |  6 +++---
> >  t/t3903-stash.sh | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 55 insertions(+), 3 deletions(-)
> > 
> > diff --git a/builtin/stash.c b/builtin/stash.c
> > index b2813c614c..9df072b459 100644
> > --- a/builtin/stash.c
> > +++ b/builtin/stash.c
> > @@ -537,7 +537,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
> >  	repo_read_index_preload(the_repository, NULL, 0);
> >  	if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
> >  					 NULL, NULL, NULL))
> > -		return -1;
> > +		return error(_("Cannot write to the index"));
> >  
> >  	if (write_index_as_tree(&c_tree, &the_index, get_index_file(), 0,
> >  				NULL))
> > @@ -1364,7 +1364,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
> >  	repo_read_index_preload(the_repository, NULL, 0);
> >  	if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
> >  					 NULL, NULL, NULL) < 0) {
> > -		ret = -1;
> > +		ret = error(_("Cannot write to the index"));
> >  		goto done;
> >  	}
> >  
> > @@ -1555,7 +1555,7 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
> >  
> >  	if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
> >  					 NULL, NULL, NULL)) {
> > -		ret = -1;
> > +		ret = error(_("Cannot write to the index"));
> >  		goto done;
> >  	}
> >  
> > diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> > index 3319240515..770881e537 100755
> > --- a/t/t3903-stash.sh
> > +++ b/t/t3903-stash.sh
> > @@ -1516,4 +1516,56 @@ test_expect_success 'restore untracked files even when we hit conflicts' '
> >  	)
> >  '
> >  
> > +test_expect_success 'stash create reports a locked index' '
> > +	test_when_finished "rm -rf repo" &&
> > +	git init repo &&
> > +	(
> > +		cd repo &&
> > +		test_commit A A.file &&
> > +		echo change >A.file &&
> > +		touch .git/index.lock &&
> > +
> > +		cat >expect <<-EOF &&
> > +		error: Cannot write to the index
> > +		EOF
> > +		test_must_fail git stash create 2>err &&
> > +		test_cmp expect err
> > +	)
> > +'
> > +
> > +test_expect_success 'stash push reports a locked index' '
> > +	test_when_finished "rm -rf repo" &&
> > +	git init repo &&
> > +	(
> > +		cd repo &&
> > +		test_commit A A.file &&
> > +		echo change >A.file &&
> > +		touch .git/index.lock &&
> > +
> > +		cat >expect <<-EOF &&
> > +		error: Cannot write to the index
> > +		EOF
> > +		test_must_fail git stash push 2>err &&
> > +		test_cmp expect err
> > +	)
> > +'
> > +
> > +test_expect_success 'stash apply reports a locked index' '
> > +	test_when_finished "rm -rf repo" &&
> > +	git init repo &&
> > +	(
> > +		cd repo &&
> > +		test_commit A A.file &&
> > +		echo change >A.file &&
> > +		git stash push &&
> > +		touch .git/index.lock &&
> > +
> > +		cat >expect <<-EOF &&
> > +		error: Cannot write to the index
> > +		EOF
> > +		test_must_fail git stash apply 2>err &&
> > +		test_cmp expect err
> > +	)
> > +'
> > +
> >  test_done
> > -- 
> > 2.43.GIT
> > 
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] builtin/stash: report failure to write to index

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 3118 bytes --]

On Mon, Feb 05, 2024 at 07:24:49PM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > The git-stash(1) command needs to write to the index for many of its
> > operations. When the index is locked by a concurrent writer it will thus
> > fail to operate, which is expected. What is not expected though is that
> > we do not print any error message at all in this case. The user can thus
> > easily miss the fact that the command didn't do what they expected it to
> > do and would be left wondering why that is.
> 
> Hopefully, they know they notice the exit status of the command, or
> do we throw the error away and exit(0) from the program?

We do return a proper exit code as demonstrated by the tests. But if you
interactively use those commands in the shell then you're quite likely
to not notice error codes at all -- my shell certainly doesn't highlight
failed commands in any special way.

> In any case, telling the users what did (and did not) happen is a
> good idea.
> 
> > Fix this bug and report failures to write to the index. Add tests for
> > the subcommands which hit the respective code paths.
> >
> > Note that the chosen error message ("Cannot write to the index") does
> > not match our guidelines as it starts with a capitalized letter. This is
> > intentional though and matches the style of all the other messages used
> > in git-stash(1).
> 
> Style may be OK, but I wonder if they should say different things,
> to hint what failed.  For example:
> 
> > @@ -537,7 +537,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
> >  	repo_read_index_preload(the_repository, NULL, 0);
> >  	if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
> >  					 NULL, NULL, NULL))
> > -		return -1;
> > +		return error(_("Cannot write to the index"));
> >
> >  	if (write_index_as_tree(&c_tree, &the_index, get_index_file(), 0,
> >  				NULL))
> 
> This failure and message comes before anything interesting happens.
> We attempted to refresh the current index and failed to write out
> the result, meaning that whatever index we had on disk did not get
> overwritten.  Is this new message enough to tell the user that we
> didn't touch the working tree or the index, which would happen if
> even some part of "stash apply" happened?  Or is it obvious that we
> did not do anything?

As a user, my expectation is that if a command failed, it didn't do
anything. If it did something before failing and wasn't able to clean it
up, then it is the responsibility of the command to tell me that it
might have screwed up and left behind some partially-applied changes.

It could certainly be that my expectation is way off. But personally, I
don't think it's useful to say "We didn't do anything" in every case
where we failed without doing anything -- I'd rather feel that it is
quite spammy.

But anyway, I know that my UX skills are severely lacking. So in case
you or anybody else has a specific suggestion for how to make it better
then I'm certainly happy to adapt.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2] builtin/stash: report failure to write to index

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 7465 bytes --]

The git-stash(1) command needs to write to the index for many of its
operations. When the index is locked by a concurrent writer it will thus
fail to operate, which is expected. What is not expected though is that
we do not print any error message at all in this case. The user can thus
easily miss the fact that the command didn't do what they expected it to
do and would be left wondering why that is.

Fix this bug and report failures to write to the index. Add tests for
the subcommands which hit the respective code paths.

While at it, unify error messages when writing to the index fails. The
chosen error message is already used in "builtin/stash.c".

Reported-by: moti sd <motisd8@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Range-diff against v1:
1:  f114688eac < -:  ---------- drop dot
2:  b13a5a10ac ! 1:  cb098cf88c builtin/stash: report failure to write to index
    @@ Commit message
         Fix this bug and report failures to write to the index. Add tests for
         the subcommands which hit the respective code paths.
     
    -    Note that the chosen error message ("Cannot write to the index") does
    -    not match our guidelines as it starts with a capitalized letter. This is
    -    intentional though and matches the style of all the other messages used
    -    in git-stash(1).
    +    While at it, unify error messages when writing to the index fails. The
    +    chosen error message is already used in "builtin/stash.c".
     
         Reported-by: moti sd <motisd8@gmail.com>
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
      ## builtin/stash.c ##
    +@@ builtin/stash.c: static void unstage_changes_unless_new(struct object_id *orig_tree)
    + 	repo_hold_locked_index(the_repository, &lock, LOCK_DIE_ON_ERROR);
    + 	if (write_locked_index(&the_index, &lock,
    + 			       COMMIT_LOCK | SKIP_IF_UNCHANGED))
    +-		die(_("Unable to write index."));
    ++		die(_("could not write index"));
    + }
    + 
    + static int do_apply_stash(const char *prefix, struct stash_info *info,
     @@ builtin/stash.c: static int do_apply_stash(const char *prefix, struct stash_info *info,
      	repo_read_index_preload(the_repository, NULL, 0);
      	if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
      					 NULL, NULL, NULL))
     -		return -1;
    -+		return error(_("Cannot write to the index"));
    ++		return error(_("could not write index"));
      
      	if (write_index_as_tree(&c_tree, &the_index, get_index_file(), 0,
      				NULL))
    @@ builtin/stash.c: static int do_create_stash(const struct pathspec *ps, struct st
      	if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
      					 NULL, NULL, NULL) < 0) {
     -		ret = -1;
    -+		ret = error(_("Cannot write to the index"));
    ++		ret = error(_("could not write index"));
      		goto done;
      	}
      
    @@ builtin/stash.c: static int do_push_stash(const struct pathspec *ps, const char
      	if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
      					 NULL, NULL, NULL)) {
     -		ret = -1;
    -+		ret = error(_("Cannot write to the index"));
    ++		ret = error(_("could not write index"));
      		goto done;
      	}
      
    @@ t/t3903-stash.sh: test_expect_success 'restore untracked files even when we hit
     +		touch .git/index.lock &&
     +
     +		cat >expect <<-EOF &&
    -+		error: Cannot write to the index
    ++		error: could not write index
     +		EOF
     +		test_must_fail git stash create 2>err &&
     +		test_cmp expect err
    @@ t/t3903-stash.sh: test_expect_success 'restore untracked files even when we hit
     +		touch .git/index.lock &&
     +
     +		cat >expect <<-EOF &&
    -+		error: Cannot write to the index
    ++		error: could not write index
     +		EOF
     +		test_must_fail git stash push 2>err &&
     +		test_cmp expect err
    @@ t/t3903-stash.sh: test_expect_success 'restore untracked files even when we hit
     +		touch .git/index.lock &&
     +
     +		cat >expect <<-EOF &&
    -+		error: Cannot write to the index
    ++		error: could not write index
     +		EOF
     +		test_must_fail git stash apply 2>err &&
     +		test_cmp expect err

 builtin/stash.c  |  8 ++++----
 t/t3903-stash.sh | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/builtin/stash.c b/builtin/stash.c
index 26489b76c0..d65cd20ee6 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -520,7 +520,7 @@ static void unstage_changes_unless_new(struct object_id *orig_tree)
 	repo_hold_locked_index(the_repository, &lock, LOCK_DIE_ON_ERROR);
 	if (write_locked_index(&the_index, &lock,
 			       COMMIT_LOCK | SKIP_IF_UNCHANGED))
-		die(_("Unable to write index."));
+		die(_("could not write index"));
 }
 
 static int do_apply_stash(const char *prefix, struct stash_info *info,
@@ -537,7 +537,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
 	repo_read_index_preload(the_repository, NULL, 0);
 	if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
 					 NULL, NULL, NULL))
-		return -1;
+		return error(_("could not write index"));
 
 	if (write_index_as_tree(&c_tree, &the_index, get_index_file(), 0,
 				NULL))
@@ -1364,7 +1364,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
 	repo_read_index_preload(the_repository, NULL, 0);
 	if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
 					 NULL, NULL, NULL) < 0) {
-		ret = -1;
+		ret = error(_("could not write index"));
 		goto done;
 	}
 
@@ -1555,7 +1555,7 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
 
 	if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
 					 NULL, NULL, NULL)) {
-		ret = -1;
+		ret = error(_("could not write index"));
 		goto done;
 	}
 
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 3319240515..00db82fb24 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -1516,4 +1516,56 @@ test_expect_success 'restore untracked files even when we hit conflicts' '
 	)
 '
 
+test_expect_success 'stash create reports a locked index' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit A A.file &&
+		echo change >A.file &&
+		touch .git/index.lock &&
+
+		cat >expect <<-EOF &&
+		error: could not write index
+		EOF
+		test_must_fail git stash create 2>err &&
+		test_cmp expect err
+	)
+'
+
+test_expect_success 'stash push reports a locked index' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit A A.file &&
+		echo change >A.file &&
+		touch .git/index.lock &&
+
+		cat >expect <<-EOF &&
+		error: could not write index
+		EOF
+		test_must_fail git stash push 2>err &&
+		test_cmp expect err
+	)
+'
+
+test_expect_success 'stash apply reports a locked index' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit A A.file &&
+		echo change >A.file &&
+		git stash push &&
+		touch .git/index.lock &&
+
+		cat >expect <<-EOF &&
+		error: could not write index
+		EOF
+		test_must_fail git stash apply 2>err &&
+		test_cmp expect err
+	)
+'
+
 test_done
-- 
2.43.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

git bug

[Justin Tang]

Thank you for filling out a Git bug report!

Please answer the following questions to help us understand your issue.


What did you do before the bug happened? (Steps to reproduce your issue)


add a new file to the git directory, untracked. with some path path/to/file


I wanted to use git diff on untracked files, so I added the file as an

empty file to git, with:


git add --intent-to-add path/to/file


Then I wanted to stash my changes, with:


git stash save


What did you expect to happen? (Expected behavior)


My changes to be stashed.


What happened instead? (Actual behavior)


I can't stash my changes, git stash save or git stash save --include-untracked.

I was getting this error:


error: Entry 'path/to/file' not uptodate. Cannot merge. Cannot save
the current worktree state


What's different between what you expected and what actually happened?


umm


Anything else you want to add:

Anything else you want to add:


I 'fixed' this by removing the empty file with git rm -r --cached path/to/file


Please review the rest of the bug report below.

You can delete any lines you don't wish to share.



[System Info]

git version:

git version 2.38.4

cpu: x86_64

no commit associated with this build

sizeof-long: 8

sizeof-size_t: 8

shell-path: /bin/sh

uname: Linux 3.10.0-1127.13.1.el7.x86_64 #1 SMP Tue Jun 23 15:46:38
UTC 2020 x86_64

compiler info: gnuc: 10.2

libc info: glibc: 2.17

$SHELL (typically, interactive shell): /bin/bash



[Enabled Hooks]

commit-msg

post-commit

post-rewrite

pre-commit

pre-push

prepare-commit-msg

Re: Git bug

[Gabriel Furstenheim Milerud]

In case someone stumbles onto this. The right way to do it is:

git rev-parse --symbolic-full-name --abbrev-ref=loose @{-1}

On Sat, 15 Apr 2023 at 11:00, Gabriel Furstenheim Milerud
<furstenheim@gmail.com> wrote:
>
> Sorry, my bad. I didn't realize that it worked based on commits. I got confused by git checkout -, which doesn't.
>
> Cheers
>
> On Fri, 14 Apr 2023, 18:07 René Scharfe, <l.s.r@web.de> wrote:
>>
>> Am 14.04.23 um 11:41 schrieb Gabriel Furstenheim Milerud:
>> > Thank you for filling out a Git bug report!
>> > Please answer the following questions to help us understand your issue.
>> >
>> > What did you do before the bug happened? (Steps to reproduce your issue)
>> >> repository in branch A
>> > git name-rev $(git rev-parse HEAD) --name-only
>> >> returns A
>> > git checkout B
>> > git name-rev $(git rev-parse HEAD) --name-only
>> >
>> >
>> >
>> > What did you expect to happen? (Expected behavior)
>> > It should return B
>> >
>> > What happened instead? (Actual behavior)
>> > It returns A
>>
>> Do you have a short recipe for creating the branches to reproduce this
>> behavior?  Here's my (failed) attempt:
>>
>>    # create repository
>>    git init -q /tmp/repo
>>    cd /tmp/repo
>>
>>    # create branch a
>>    echo a >a
>>    git add a
>>    git commit -q -m a
>>    git branch a
>>
>>    # create branch b
>>    echo b >b
>>    git add b
>>    git commit -q -m b
>>    git branch b
>>
>> With that done Git v2.34.1 gives me the output you expect:
>>
>>    $ git switch a
>>    Switched to branch 'a'
>>    $ git name-rev $(git rev-parse HEAD) --name-only
>>    a
>>
>>    $ git switch b
>>    Switched to branch 'b'
>>    $ git name-rev $(git rev-parse HEAD) --name-only
>>    b
>>
>> > What's different between what you expected and what actually happened?
>> > git rev-parse does not seem to update. Same is happenning with git
>> > rev-parse @{-1}
>>
>>    $ git name-rev $(git rev-parse @{-1}) --name-only
>>    a
>>
>> This is expected because @{-1} means the branch/commit checked out
>> before the current one.
>>
>> What does "git rev-parse HEAD" return for you in each case?  Do your
>> branches perhaps have the same HEAD commit?
>>
>> > Anything else you want to add:
>> > git version 2.34.1
>> >
>> > I recently migrated to ubuntu 22. In ubuntu 18 with previous git
>> > version is was working as expected.
>>
>> I guess you use LTS releases, i.e. jammy (22.04) and bionic (18.04)?
>> bionic shipped with Git 2.17.1 according to https://packages.ubuntu.com/
>>
>> René
>>

Re: Git bug

[René Scharfe]

Am 14.04.23 um 11:41 schrieb Gabriel Furstenheim Milerud:
> Thank you for filling out a Git bug report!
> Please answer the following questions to help us understand your issue.
>
> What did you do before the bug happened? (Steps to reproduce your issue)
>> repository in branch A
> git name-rev $(git rev-parse HEAD) --name-only
>> returns A
> git checkout B
> git name-rev $(git rev-parse HEAD) --name-only
>
>
>
> What did you expect to happen? (Expected behavior)
> It should return B
>
> What happened instead? (Actual behavior)
> It returns A

Do you have a short recipe for creating the branches to reproduce this
behavior?  Here's my (failed) attempt:

   # create repository
   git init -q /tmp/repo
   cd /tmp/repo

   # create branch a
   echo a >a
   git add a
   git commit -q -m a
   git branch a

   # create branch b
   echo b >b
   git add b
   git commit -q -m b
   git branch b

With that done Git v2.34.1 gives me the output you expect:

   $ git switch a
   Switched to branch 'a'
   $ git name-rev $(git rev-parse HEAD) --name-only
   a

   $ git switch b
   Switched to branch 'b'
   $ git name-rev $(git rev-parse HEAD) --name-only
   b

> What's different between what you expected and what actually happened?
> git rev-parse does not seem to update. Same is happenning with git
> rev-parse @{-1}

   $ git name-rev $(git rev-parse @{-1}) --name-only
   a

This is expected because @{-1} means the branch/commit checked out
before the current one.

What does "git rev-parse HEAD" return for you in each case?  Do your
branches perhaps have the same HEAD commit?

> Anything else you want to add:
> git version 2.34.1
>
> I recently migrated to ubuntu 22. In ubuntu 18 with previous git
> version is was working as expected.

I guess you use LTS releases, i.e. jammy (22.04) and bionic (18.04)?
bionic shipped with Git 2.17.1 according to https://packages.ubuntu.com/

René

Git bug

[Gabriel Furstenheim Milerud]

Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.

What did you do before the bug happened? (Steps to reproduce your issue)
> repository in branch A
git name-rev $(git rev-parse HEAD) --name-only
> returns A
git checkout B
git name-rev $(git rev-parse HEAD) --name-only



What did you expect to happen? (Expected behavior)
It should return B

What happened instead? (Actual behavior)
It returns A

What's different between what you expected and what actually happened?
git rev-parse does not seem to update. Same is happenning with git
rev-parse @{-1}

Anything else you want to add:
git version 2.34.1

I recently migrated to ubuntu 22. In ubuntu 18 with previous git
version is was working as expected.

Please review the rest of the bug report below.
You can delete any lines you don't wish to share.


[System Info]
git version:
git version 2.34.1
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname: Linux 5.19.0-38-generic #39~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC
Fri Mar 17 21:16:15 UTC 2 x86_64
compiler info: gnuc: 11.3
libc info: glibc: 2.35
$SHELL (typically, interactive shell): /bin/bash


[Enabled Hooks]

Re: Git bug.

[Kristoffer Haugsbakk]

On Fri, Apr 14, 2023, at 10:29, Gabriel Furstenheim Milerud wrote:
> Hi,
> Sorry if this is not the right address to report bugs. I'm following
> https://stackoverflow.com/questions/10728104/where-can-i-report-a-git-bug/10733251#10733251

LGTM.

VonC has amazing Git answers on SO, but I feel that this changelog-type
answer is *too much* for a procedure which is just (based on the bug
reports from the last month or so):

1. Run `git bugreport`
2. Edit the file by filling out the blanks and removing irrelevant stuff
   (follow the instructions)
3. Post the report as an inline plaintext email

So basically the sibling answer https://stackoverflow.com/a/70175922/1725151

-- 
Kristoffer Haugsbakk

Git bug.

[Gabriel Furstenheim Milerud]

[-- Attachment #1: Type: text/plain, Size: 200 bytes --]

Hi,
Sorry if this is not the right address to report bugs. I'm following
https://stackoverflow.com/questions/10728104/where-can-i-report-a-git-bug/10733251#10733251

Cheers
Gabriel Fürstenheim

[-- Attachment #2: git-bugreport-2023-04-14-1018.txt --]
[-- Type: text/plain, Size: 1239 bytes --]

Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.

What did you do before the bug happened? (Steps to reproduce your issue)
> repository in branch A
git name-rev $(git rev-parse HEAD) --name-only
> returns A
git checkout B
git name-rev $(git rev-parse HEAD) --name-only



What did you expect to happen? (Expected behavior)
It should return B

What happened instead? (Actual behavior)
It returns A

What's different between what you expected and what actually happened?
git rev-parse does not seem to update. Same is happenning with git rev-parse @{-1}

Anything else you want to add:
git version 2.34.1

I recently migrated to ubuntu 22. In ubuntu 18 with previous git version is was working as expected.

Please review the rest of the bug report below.
You can delete any lines you don't wish to share.


[System Info]
git version:
git version 2.34.1
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname: Linux 5.19.0-38-generic #39~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Fri Mar 17 21:16:15 UTC 2 x86_64
compiler info: gnuc: 11.3
libc info: glibc: 2.35
$SHELL (typically, interactive shell): /bin/bash


[Enabled Hooks]

Re: git bug

[PANEL Christian]

thanks a lot.
It was exactly what I was seaching (and havent' read yet ;) )

best regards

Le lundi 10 août 2020 à 07:56 +0200, René Scharfe a écrit :
> Am 07.08.20 um 02:02 schrieb Jeff King:
> > 
> > On Thu, Aug 06, 2020 at 10:23:54PM +0200, René Scharfe wrote:
> > 
> > > 
> > > So "file" is no longer ignored.  Committing the .gitignore change
> > > doesn't change that:
> > > 
> > >   $ git add .gitignore
> > >   $ git commit -m 2nd
> > >   [master d4c95a1] 2nd
> > >    1 file changed, 1 deletion(-)
> > >   $ git status
> > >   On branch master
> > >   Untracked files:
> > >     (use "git add <file>..." to include in what will be committed)
> > >   	file
> > > 
> > >   nothing added to commit but untracked files present (use "git add" to track)
> > > 
> > > Which steps did you take to arrive at a different result?
> > Perhaps also:
> > 
> >   git check-ignore -v file
> > 
> > would be helpful for seeing why Git thinks it might be ignored (e.g.,
> > another wildcard rule that happens to match it).
> Right.  And there is more than one possible place to specify files to be
> ignored.  E.g. you can use info/exclude in your Git directory (i.e.
> .git/info/exclude by default) for repository-specific patterns don't
> want to share.  See https://git-scm.com/docs/gitignore or the manpage
> of gitignore(5) for more details.
> 
> René

Re: git bug

[René Scharfe]

Am 07.08.20 um 02:02 schrieb Jeff King:
> On Thu, Aug 06, 2020 at 10:23:54PM +0200, René Scharfe wrote:
>
>> So "file" is no longer ignored.  Committing the .gitignore change
>> doesn't change that:
>>
>>   $ git add .gitignore
>>   $ git commit -m 2nd
>>   [master d4c95a1] 2nd
>>    1 file changed, 1 deletion(-)
>>   $ git status
>>   On branch master
>>   Untracked files:
>>     (use "git add <file>..." to include in what will be committed)
>>   	file
>>
>>   nothing added to commit but untracked files present (use "git add" to track)
>>
>> Which steps did you take to arrive at a different result?
>
> Perhaps also:
>
>   git check-ignore -v file
>
> would be helpful for seeing why Git thinks it might be ignored (e.g.,
> another wildcard rule that happens to match it).

Right.  And there is more than one possible place to specify files to be
ignored.  E.g. you can use info/exclude in your Git directory (i.e.
.git/info/exclude by default) for repository-specific patterns don't
want to share.  See https://git-scm.com/docs/gitignore or the manpage
of gitignore(5) for more details.

René

Re: git bug

[Jeff King]

On Thu, Aug 06, 2020 at 10:23:54PM +0200, René Scharfe wrote:

> So "file" is no longer ignored.  Committing the .gitignore change
> doesn't change that:
> 
>   $ git add .gitignore
>   $ git commit -m 2nd
>   [master d4c95a1] 2nd
>    1 file changed, 1 deletion(-)
>   $ git status
>   On branch master
>   Untracked files:
>     (use "git add <file>..." to include in what will be committed)
>   	file
> 
>   nothing added to commit but untracked files present (use "git add" to track)
> 
> Which steps did you take to arrive at a different result?

Perhaps also:

  git check-ignore -v file

would be helpful for seeing why Git thinks it might be ignored (e.g.,
another wildcard rule that happens to match it).

-Peff

Re: git bug

[René Scharfe]

Am 06.08.20 um 16:48 schrieb PANEL Christian:
> someting is not logical in the behaviour of git :
>
> suppose I have a project and a file in it I don't want to include initially in.
> so I put this file name in .gitignore
> now all is OK when I write "git status" : the file is ignored.

Like this, right?

  $ git init a
  Initialized empty Git repository in /tmp/a/.git/
  $ cd a
  $ echo ignore for now >file
  $ echo file >.gitignore
  $ git add .gitignore
  $ git commit -m initial
  [master (root-commit) 2df5d68] initial
   1 file changed, 1 insertion(+)
   create mode 100644 .gitignore
  $ git status
  On branch master
  nothing to commit, working tree clean

> but later I want this one be a part of my project.
> I delete in .gitignore the line that named this file.
> but now a "git status" command ignore always this file.
>
> what is wrong ?
> did I missed something ?

This seems to work as expected for me (continued from above):

  $ >.gitignore
  $ git status
  On branch master
  Changes not staged for commit:
    (use "git add <file>..." to update what will be committed)
    (use "git restore <file>..." to discard changes in working directory)
  	modified:   .gitignore

  Untracked files:
    (use "git add <file>..." to include in what will be committed)
  	file

  no changes added to commit (use "git add" and/or "git commit -a")

So "file" is no longer ignored.  Committing the .gitignore change
doesn't change that:

  $ git add .gitignore
  $ git commit -m 2nd
  [master d4c95a1] 2nd
   1 file changed, 1 deletion(-)
  $ git status
  On branch master
  Untracked files:
    (use "git add <file>..." to include in what will be committed)
  	file

  nothing added to commit but untracked files present (use "git add" to track)

Which steps did you take to arrive at a different result?

René

git bug

[PANEL Christian]

someting is not logical in the behaviour of git :

suppose I have a project and a file in it I don't want to include initially in.
so I put this file name in .gitignore
now all is OK when I write "git status" : the file is ignored.

but later I want this one be a part of my project.
I delete in .gitignore the line that named this file.
but now a "git status" command ignore always this file.

what is wrong ?
did I missed something ?

thanks for any answer 

git bug

[PANEL Christian]

someting is not logical in the behaviour of git :

suppose I have a project and a file in it I don't want to include initially in.
so I put this file name in .gitignore
now all is OK when I write "git status" : the file is ignored.

but later I want this one be a part of my project.
I delete in .gitignore the line that named this file.
but now a "git status" command ignore always this file.

what is wrong ?
did I missed something ?

thanks for any answer 

git BUG

[Коньков Евгений]

Hi, git

I have get git bug while merging branches.

the farm has branch one year old. say b1
I have develop some feature in this year so I start new branch
  git checkout master
  git checkout -b b2

I get task to develop feature b1. b1 require changes in b2
  git checkout b1
  git merge b2
  git checkout --theirs
  git commit

after some time I need new code from master
  git merge master

after this merge I notice that some code, that was developed in b2, is
disappeared despite on it was merged into b1

so I start to merge it again:
  git merge b2

No results. So I decide to do some testing. I go to b2
  git checkout b2

make changes into file on b2:
diff --git a/lib/SRS/Domain.pm b/lib/SRS/Domain.pm
index 8fa1b1a..23a9429 100644
--- a/lib/SRS/Domain.pm
+++ b/lib/SRS/Domain.pm
@@ -2804,6 +2804,7 @@ sub can_renew_state {
 sub zone {
     my ( $self ) =  @_;

+
     load_class 'SRS::Comm::Zone::Domain';

     return

I have add one empty line to subroutine that have developed in b2.
  git commit
  git checkout b1
  git merge b2

and I get conflict:
+++ b/lib/SRS/Domain.pm
@@@ -2830,24 -2801,20 +2830,29 @@@ sub can_renew_state

  =cut

 -sub zone {
 -    my ( $self ) =  @_;
 +sub can_cancel_transin {
 +    my $self = shift;

++<<<<<<< HEAD
 +    return if $self->{freeze_date};
 +    return unless $self->{create_method} eq 'trans' && $self->{state} eq 'N';
++=======
+
+     load_class 'SRS::Comm::Zone::Domain';
++>>>>>>> yandex_mail_new_api

 -    return
 -        SRS::Comm::Zone::Domain->new(
 -            dname   => $self->{dname},
 -            service => $self,
 -        );
 +    my ( $pos_id ) = dbh_rw->selectrow_array(
 +        'SELECT bi.pos_id FROM billitems bi JOIN bills b ON b.bill_id = bi.bill_id WHERE service_id =
 +        undef,
 +        $self->{service_id}, $self->{user_id}
 +    );
 +
 +    return $pos_id;
  }

 -=back
 +=item cancel_transin
 +
 +Ф-ция отмены переноса

  =cut


As you can see conflict at 'can_cancel_transin' subroutine
but I do changes in 'zone' subroutine


Please help to resolve that CONFLICT and return code from b2 to be
alive again in b1.

And, if you can, please explain why this occur

-- 
 С уважением,
 Коньков Евгений
 Программист
 Регистратор доменных имен REG.RU
 Телефон: +38 (097) 7654-676
 www.reg.ru
 ___________________
 Sincerely yours,
 Konkov Eugen
 Developer
 Accredited domain Registrar REG.RU, LLC.
 Phone: +38 (097) 7654-676
 www.reg.ru/en/

mailto:kes@reg.ru