On Thu, Jan 19, 2017 at 8:00 PM, Junio C Hamano <gits...@pobox.com> wrote: > Duy Nguyen <pclo...@gmail.com> writes: > >> On Mon, Jan 9, 2017 at 9:34 PM, Junio C Hamano <gits...@pobox.com> wrote: >>> Duy Nguyen <pclo...@gmail.com> writes: >>> >>>> On Sun, Jan 8, 2017 at 4:46 AM, Junio C Hamano <gits...@pobox.com> wrote: >>>>> Christian Couder <christian.cou...@gmail.com> writes: >>>>> >>>>>> So what should we do if freshen_file() returns 0 which means that the >>>>>> freshening failed? >>>>> >>>>> You tell me ;-) as you are the one who is proposing this feature.
Before the above question I already had given my opinion about what we should do. There are the following cases: - Freshening failed because the shared index file does not exist anymore. In this case it could have been removed for a good reason (for example maybe the user wants to remove all the shared index files), or it could be a bug somewhere else. Anyway we cannot know and the user will get an error if the shared index file that disappeared is read from afterwards, so I don't think we need to warn or do anything. - Freshening failed because the mtime of the shared index cannot be changed. You already in a previous email wrote that we shoudn't warn if the file system is read only, and I agree with that, as anyway if the file system is read only, the shared index file cannot be deleted, so there is no risk from the current user. Also the split index mode is useful to speed up index writing at the cost of making index reading a little slower, so its use in a read only mode should not be the primary way it is used. So my opinion is that there are good reasons not to do anything if freshening fails. >>>> My answer is, we are not worse than freshening loose objects case >>>> (especially since I took the idea from there). >>> >>> I do not think so, unfortunately. Loose object files with stale >>> timestamps are not removed as long as objects are still reachable. As the current index is read, which freshens its shared index file, before a new index is created, the number of shared index files doesn't go below 2. This can be seen in the tests changed in patch 19/21. So the risk of deleting interesting shared index files is quite low in my opinion. Also in general the split-index mode is useful when you often write new indexes, and in this case shared index files that are used will often be freshened, so the risk of deleting interesting shared index files should be low. >> But there are plenty of unreachable loose objects, added in index, >> then got replaced with new versions. cache-tree can create loose trees >> too and it's been run more often, behind user's back, to take >> advantage of the shortcut in unpack-trees. > > I am not sure if I follow. Aren't objects reachable from the > cache-tree in the index protected from gc? > > Not that I think freshening would actually fail in a repository > where you can actually write into to update the index or its refs to > make a difference (iow, even if we make it die() loudly when shared > index cannot be "touched" because we are paranoid, no real life > usage will trigger that die(), and if a repository does trigger the > die(), I think you would really want to know about it). As I wrote above, I think if we can actually write the shared index file but its freshening fails, it probably means that the shared index file has been removed behind us, and this case is equivalent as when loose files have been removed behind us.
