cms-sw / cms-git-tools

CMS Git Helpers
34 stars 26 forks source link

git-cms-addpkg resets staged (but not yet commited) changes without a warning. #57

Closed dmitrijus closed 10 years ago

dmitrijus commented 10 years ago

Hi,

If you do git-cms-addpkg, all staged (but not yet committed) changes will be reset without a warning.

> git-cms-addpkg RecoLocalCalo
> touch RecoLocalCalo/test
> git add RecoLocalCalo/test
> git status
# Changes to be committed:
#   (use "git reset HEAD <file>..." to unstage)
#
#   new file:   RecoLocalCalo/test
#
> git-cms-addpkg EventFilter
> ls -la RecoLocalCalo/test
ls: RecoLocalCalo/test: No such file or directory
> git status
nothing to commit, working directory clean

Dmitrijus

dmitrijus commented 10 years ago

I've accidentally misclicked and closed, reopening.

fwyzard commented 10 years ago

I can confirm the issue.

@ktf , maybe we can wrap the git-cms-addpkg internals with a git stash ? I.e. something like

git stash save --keep-index --include-untracked "prepare for git cms-addpkg" ...do whatever git cms-addpkg was doing git stash pop stash@{0}

?

dmitrijus commented 10 years ago

I don't think stashing is a good solution and it just might cause complex bugs in the future.

A simple check would be enough, quick-n-dirty: [[ -z $(git status --porcelain) ]] || exit 1

Note: read-tree already fails with unmerged files.

rovere commented 10 years ago

Hey, I just jumped into the very same problem and I'd vote for the stash solution. @dmitrijus what are the drawbacks of this approach you see?

M.

dmitrijus commented 10 years ago

I have nothing against stashing except that it adds complexity to the wrapper script.

My solution is just to check for staged files, without doing any damage. I believe developers who are hit by this bug will know how to resolve this issue.

Additionally, this bug might appear outside the situation I've described. I've seen it while doing merging and/or rebasing (if you have a conflict, git puts already merged files into staging). I am not sure if you can stash unfinished merges/rebases correctly, I don't know git that well.

fwyzard commented 10 years ago

I've added the simple check to both git-cms-addpkg and git-cms-merge-topic in #65 .

dmitrijus commented 10 years ago

Okay, thanks.