Zygo / bees

Best-Effort Extent-Same, a btrfs dedupe agent
GNU General Public License v3.0
630 stars 57 forks source link

Bees won't start anymore because of grep usage error in the starting script #233

Closed marcin-rzeznicki closed 1 year ago

marcin-rzeznicki commented 1 year ago

This hasty change https://github.com/Zygo/bees/commit/dffd6e0b13f9f049f45f7ae5bf7e64fa71804646 by @KhalilSantana basically killed beesd because, after this, when you try to start it you get:

beesd[4118]: Usage: grep [OPTION]... PATTERNS [FILE]...
beesd[4118]: Try 'grep --help' for more information.
beesd[4120]: uuidparse: unrecognized option '--no-timestamps'
beesd[4120]: Try 'uuidparse --help' for more information.

The reason being that grep -E "--" does this:

$ /usr/lib/bees/bees --help 2>&1 | grep -E "--"
Usage: grep [OPTION]... PATTERNS [FILE]...
Try 'grep --help' for more information.

whereas the old one worked fine (albeit with warning):

$ /usr/lib/bees/bees --help 2>&1 | grep -E "\-\-"
grep: warning: stray \ before -
-h, --help            Show this help
-c, --thread-count    Worker thread count (default CPU count * factor)
-C, --thread-factor   Worker thread factor (default 1)
-G, --thread-min      Minimum worker thread count (default 0)
-g, --loadavg-target  Target load average for worker threads (default none)
-m, --scan-mode       Scanning mode (0..2, default 0)
-a, --workaround-btrfs-send    Workaround for btrfs send
-t, --timestamps      Show timestamps in log output (default)
-T, --no-timestamps   Omit timestamps in log output
-p, --absolute-paths  Show absolute paths (default)
-P, --strip-paths     Strip $CWD from beginning of all paths in the log
-v, --verbose         Set maximum log level (0..8, default 8)

I will refrain from commenting on the addition of an obviously untested commit a day before the release.

KhalilSantana commented 1 year ago

Hum, odd I do believe I've tested that commit, maybe I forgot to set pacman to override the file and/or fixed it up quickly without commiting again?

Anyways, the fix is rather simple and I've submitted PR #234 to address this on the master branch, please test if it works properly this time (I've double checked locally and it works on my machine at least).

marcin-rzeznicki commented 1 year ago

Thanks a lot for your very prompt response! Much appreciated!

kakra commented 1 year ago

Hum, odd I do believe I've tested that commit

TBF, locally on my machine, there was also no error with this change. It seems there are different versions of grep in the wild with different behavior. Claiming a change was untested is thus at least a little unfair:

I will refrain from commenting on the addition of an obviously untested commit a day before the release.

It is actually even not "obvious" that this was untested. It could be argued instead, tho, that this exact change was kinda useless as -E does not make a real difference here, so maybe better not change a working part of a script.

Just my two cents. ;-)

marcin-rzeznicki commented 1 year ago

@kakra I was referring to changing the pattern which can have bad consequences, as we've seen. But you're right, I should not have assumed that. Sorry for the unjustified words and thanks to everyone for fixing it so quickly.