Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
dmitrijus opened this issue Mar 12, 2014 · 7 comments

Comments

@dmitrijus
Copy link

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
Copy link
Author

I've accidentally misclicked and closed, reopening.

@dmitrijus dmitrijus reopened this Mar 12, 2014
@fwyzard
Copy link
Contributor

fwyzard commented Mar 12, 2014

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
Copy link
Author

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
Copy link

rovere commented Jul 4, 2014

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
Copy link
Author

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
Copy link
Contributor

fwyzard commented Jul 14, 2014

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

@dmitrijus
Copy link
Author

Okay, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants