[PATCH 2/7] rebase -i: don't error out if $state_dir already exists
In preparation for a later patch that will create $state_dir/autostash in git-rebase.sh before anything else can happen, change a `mkdir $state_dir` call to `mkdir -p $state_dir`. The change is safe, because this is not a test to detect an in-progress rebase (that is already done much earlier in git-rebase.sh). Signed-off-by: Ramkumar Ramachandra --- git-rebase--interactive.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 5822b2c..3411139 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -842,7 +842,7 @@ then fi orig_head=$(git rev-parse --verify HEAD) || die "No HEAD?" -mkdir "$state_dir" || die "Could not create temporary $state_dir" +mkdir -p "$state_dir" || die "Could not create temporary $state_dir" : > "$state_dir"/interactive || die "Could not mark as interactive" write_basic_state -- 1.8.3.rc1.51.gd7a04de -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/7] rebase -i: don't error out if $state_dir already exists
Junio C Hamano wrote: > So I do not see why this change could be an improvement. I would > understand it if some future changes may have already created the > state dir from a separate codepath (which may be an indication of a > poor design of integrating that separate codepath, but that is a > different matter) and mkdir needs to be turned into "mkdir -p" here, > but I do not think removal of "|| die" is warranted even in such a > case. Yeah, it's [7/7] creating a $state_dir/autostash before anything else can happen. Okay, if I put the "|| die" back in, does it require any more justification? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/7] rebase -i: don't error out if $state_dir already exists
Ramkumar Ramachandra writes: > Practically speaking, the only reason why a `mkdir $state_dir` would > fail is because $state_dir already exists. There is no problem in > this case, and we can proceed as usual. That was started as a cheap way to detect and avoid running the same rebase -i while another one is already in progress, way back when we did not have a separate "git-rebase--*backend*" scripts, I think. If we have a separate safety measure to prevent it, lifting it may be safe, but in a sane case, $state_dir should _not_ exist when we start the command, and mkdir should _not_ fail to create it. So I do not see why this change could be an improvement. I would understand it if some future changes may have already created the state dir from a separate codepath (which may be an indication of a poor design of integrating that separate codepath, but that is a different matter) and mkdir needs to be turned into "mkdir -p" here, but I do not think removal of "|| die" is warranted even in such a case. > So, change the `mkdir` call > to `mkdir -p`, and strip the `|| die`. > > Signed-off-by: Ramkumar Ramachandra > --- > git-rebase--interactive.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh > index 048a140..cc3a9a7 100644 > --- a/git-rebase--interactive.sh > +++ b/git-rebase--interactive.sh > @@ -837,7 +837,7 @@ then > fi > > orig_head=$(git rev-parse --verify HEAD) || die "No HEAD?" > -mkdir "$state_dir" || die "Could not create temporary $state_dir" > +mkdir -p "$state_dir" > > : > "$state_dir"/interactive || die "Could not mark as interactive" > write_basic_state -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/7] rebase -i: don't error out if $state_dir already exists
On Tue, Apr 23, 2013 at 7:01 AM, Ramkumar Ramachandra wrote: > Practically speaking, the only reason why a `mkdir $state_dir` would > fail is because $state_dir already exists. Would we ever get to this point in the code if it already exists? Also, I had the feeling that the check it might fail if the user does not have permissions to create the directory. Is there always something else that will fail first? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/7] rebase -i: don't error out if $state_dir already exists
Practically speaking, the only reason why a `mkdir $state_dir` would fail is because $state_dir already exists. There is no problem in this case, and we can proceed as usual. So, change the `mkdir` call to `mkdir -p`, and strip the `|| die`. Signed-off-by: Ramkumar Ramachandra --- git-rebase--interactive.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 048a140..cc3a9a7 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -837,7 +837,7 @@ then fi orig_head=$(git rev-parse --verify HEAD) || die "No HEAD?" -mkdir "$state_dir" || die "Could not create temporary $state_dir" +mkdir -p "$state_dir" : > "$state_dir"/interactive || die "Could not mark as interactive" write_basic_state -- 1.8.2.1.578.ga933817 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
