Don't barf on duplicate recent repo list entries, nor a full list #10

PhilipOakley · 2017-01-20T21:46:13Z

git-for-windows/git#1014 highlighted this problem which I had addressed at the end of 2015, but got lost over the Christmas.

This resurects and improves my old code, so that no matter how created dulicate entries in the recent repo list do not crash the Git-gui.

The git gui's recent repo list may become contaminated with duplicate entries. The git gui would barf when attempting to remove one entry. Remove them all - there is no option within 'git config' to selectively remove one of the entries. This issue was reported on the 'Git User' list (https://groups.google.com/forum/#!topic/git-users/msev4KsQGFc, Warning: gui.recentrepo has multiply values while executing). And also by zosrothko as a Git-for-Windows issue git-for-windows/git#1014. On startup the gui checks that entries in the recentrepo list are still valid repos and deletes thoses that are not. If duplicate entries are present the 'git config --unset' will barf and this prevents the gui from starting. Subsequent patches fix other parts of recentrepo logic used for syncing internal lists with the external .gitconfig. Reported-by: Alexey Astakhov <[email protected]> Signed-off-by: Philip Oakley <[email protected]>

_get_recentrepo will fail if duplicate invalid entries are present in the recentrepo config list. The previous commit fixed the 'git config' limitations in _unset_recentrepo by unsetting all config entries, however this code would fail on the second attempt to unset it. Refactor the code to pre-sort and de-duplicate the recentrepo list to avoid a potential second unset attempt. Signed-off-by: Philip Oakley <[email protected]>

When the gui/user selects a repo for display, that repo is brought to the end of the recentrepo config list. The logic can fail if there are duplicate old entries for the repo (you cannot unset a single config entry when duplicates are present). Similarly, the maxrecentrepo logic could fail if older duplicate entries are present. The first commit of this series ({this}~2) fixed the config unsetting issue. Rather than manipulating a local copy of the $recent list (one cannot know how many entries were removed), simply re-read it. We must also catch the error when the attempt to remove the second copy from the re-read list is performed. Signed-off-by: Philip Oakley <[email protected]>

The gui.recentrepo list may be longer than the maxrecent setting. Allow extra space to show any extra entries. In an ideal world, the git gui would limit the number of entries to the maxrecent setting, however the recentrepo config list may have been extended outwith the gui, or the maxrecent setting changed to a reduced value. Further, when testing the gui's recentrepo logic it is useful to show these extra, but valid, entries. Signed-off-by: Philip Oakley <[email protected]>

dscho · 2017-02-07T12:30:53Z

@PhilipOakley seeing as @patthoyts is probably too busy to have a look at this any time soon, do you think we should go ahead and merge it into Git for Windows to unblock our users?

PhilipOakley · 2017-02-07T13:46:15Z

Yes.

I was also going to comment on the upstream Git list that this hadn't been picked up their either. There is a comment there about some gitk issues as well that haven't been picked up

This resolves a couple of Git GUI issues that seem not to have been picked up from patthoyts/git-gui#10 yet. Signed-off-by: Johannes Schindelin <[email protected]>

A couple of Git GUI bugs regarding the list of recent repositories [have been fixed](patthoyts/git-gui#10). Signed-off-by: Johannes Schindelin <[email protected]>

This resolves a couple of Git GUI issues that seem not to have been picked up from patthoyts/git-gui#10 yet. Signed-off-by: Johannes Schindelin <[email protected]>

Philip Oakley added 4 commits January 20, 2017 20:55

PhilipOakley mentioned this pull request Jan 20, 2017

critical: 2.10.2.windows.1: git-gui crashes when opening a 3rd window git-for-windows/git#1014

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't barf on duplicate recent repo list entries, nor a full list #10

Don't barf on duplicate recent repo list entries, nor a full list #10

PhilipOakley commented Jan 20, 2017

dscho commented Feb 7, 2017

PhilipOakley commented Feb 7, 2017

Don't barf on duplicate recent repo list entries, nor a full list #10

Don't barf on duplicate recent repo list entries, nor a full list #10

Conversation

PhilipOakley commented Jan 20, 2017

dscho commented Feb 7, 2017

PhilipOakley commented Feb 7, 2017