Speedup osc status

[bmwiedemann]

The 2nd commit is the main change with a small semantic change
allowing osc status on a openSUSE:Factory checkout to finish in 34 seconds instead of 730-1100 seconds.

The other commits are optional improvements.

[marcus-h]

On 2017-07-24 21:51:09 -0700, Bernhard M. Wiedemann wrote: The 2nd commit is the main change with a small semantic change allowing `osc status` on a openSUSE:Factory checkout to finish in 34 seconds

Nice! However, I'm a bit hesitant merging it in the current state, because, as you said, it changes the semantics (though, I think it'll work for almost all users...). What about introducing a config option for this?

-- Commit Summary -- * cleanup status code * speedup osc status * status: optimize some more

The last commit seems to be gone.

[bmwiedemann]

yes, I dropped the last commit after having to change the 2nd commit, because the update tests failed when I assumed that changed sizes mean "M" status.

Introducing a config option would make the code more complex and decrease the test-coverage.

The only way I know to change files without the timestamp being modified is using loop devices (e.g. via losetup or mount -o loop) but I do not think that is used on source files.

The other would be an explicit touch -d or -r but then the user probably knows what he is doing.

[bmwiedemann]

in any case, you could already review/merge the cleanup commit so that I will not have to rebase as much, in case it takes longer (called 'bitrot').

[marcus-h]

On 2017-07-26 01:39:53 -0700, Bernhard M. Wiedemann wrote: yes, I dropped the last commit after having to change the 2nd commit, because the update tests failed when I assumed that changed sizes mean "M" status. Introducing a config option would make the code more complex and decrease the test-coverage.

Hmm I don't think it will be that complex... wrt. to test-coverage: we could run the testsuite with different oscrc files (but I'm fine with running the testsuite with the option's default value (regardless if we make the mtime thing the new default)).

The only way I know to change files without the timestamp being modified is using loop devices (e.g. via losetup or mount -o loop) but I do not think that is used on source files. The other would be an explicit touch -d or -r but then the user probably knows what he is doing.

Actually, I'm more concerned about scripts. For instance, having a script, which does something like ... # commit osc ci -m foo # copy updated files over to the wc cp /path/to/files/* . # files changed, but mtimes might be the same ... can be potentially racy (depending on the filesystem's mtime granularity (struct super_block.s_time_gran)). I was able to produce such a race with api.o.o and reiserfs (which just considers seconds and ignores nsecs) (admittedly, reiserfs is old, but I still have some (rarely used) partition around:) ). PS. does anyone know by chance if it is possible to get a notification in case a PR comment was edited? (it was pure luck that I saw the second part of this comment).

[marcus-h]

On 2017-07-26 13:32:42 +0000, Bernhard M. Wiedemann wrote: in any case, you could already review/merge the cleanup commit so that I will not have to rebase as much, in case it takes longer (called 'bitrot').

Naw... in general I like your change. I just want to avoid breaking existing(?) use-cases. Hence, I'm still in favour of a config option, but maybe I'm just too conservative:) @adrianschroeter @lethiel any opinions?:)

[marcus-h]

On 2017-07-27 14:14:12 +0200, Marcus Hüwe wrote: @adrianschroeter @lethiel any opinions?:)

I mean @lethliel...

[bmwiedemann]

I added a boolean to always calculate digests again - but not integrated into config files / CLI options. can you do that?

[marcus-h]

On 2017-08-12 20:13:44 +0000, Bernhard M. Wiedemann wrote: I added a boolean to always calculate digests again - but not integrated into config files / CLI options. can you do that?

Yep, I'll take care of it. Thanks!

[marcus-h]

Merged. The mtime heuristic can be enabled via osc config general status_mtime_heuristic 1 (for the details see commit 48a35fe). Thanks a lot for the PR!