git-subrepo icon indicating copy to clipboard operation
git-subrepo copied to clipboard

upstream HEAD commit not printed when upstream_head_commit not found in rev-list

Open jrandall opened this issue 3 years ago • 0 comments

During a subrepo push, when upstream_head_commit is not found in the rev-list, the error message printed does not contain the actual commit ID, leading to confusion as it seems like it is saying this variable is empty, but it is not.

I.e. what is printed looks something like this: git-subrepo: Can't commit: 'subrepo/subrepo_name doesn't contain upstream HEAD:

But in actuality what should have been printed is this: git-subrepo: Can't commit: 'subrepo/subrepo_name doesn't contain upstream HEAD: df09ceebe685714a11e628f0c797b9e3d280e130

The problem is here: https://github.com/ingydotnet/git-subrepo/blob/master/lib/git-subrepo#L660-L661

Which calls: https://github.com/ingydotnet/git-subrepo/blob/master/lib/git-subrepo#L1919-L1922

The issue is that the error() function is being called with two arguments:

  • "Can't commit: '$branch_name' doesn't contain upstream HEAD: "
  • "$upstream_head_commit"

But the error() function ignores all but the first argument, so only the first part of the error message is printed.

I can confirm two valid fixes for this, one of which is to fix the call to error so that it only passes a single argument to the error() function. I.e. something like:

      error "Can't commit: '$branch_name' doesn't contain upstream HEAD: \                                                                                                                                         
         $upstream_head_commit"     

The other fix is to change the error() function so that it passes along all of its arguments to echo:

error() {
  echo -e "git-subrepo:" $@ >&2
  exit 1
}

Either one works, but I'd note that a quick look at other invocations of error() indicates there is at least one other situation that also passes multiple arguments to error (all but the first of which would be ignored), so the latter fix would fix all of them at once and should probably be preferred. (see https://github.com/ingydotnet/git-subrepo/blob/master/lib/git-subrepo#L1449-L1450)

jrandall avatar Jan 25 '21 18:01 jrandall