coreos-overlay-diff.py is broken with Python sh 2.x

[chewi]

Description

This script was written against sh 1.x. There were some significant breaking changes in 2.x, notably the way pipes are constructed and the type returned.

Impact

Environment and steps to reproduce

1. Set-up: I was running vanilla Gentoo with the latest dev-python/sh. Fedora 39 and 40 also have 2.x.
2. Task: Simply running coreos-overlay-diff.py.
3. Action(s): As above.
4. Error: After the diff, you get an error like this:

↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑ Diff for flatcar-overlay ↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑

Traceback (most recent call last):
  File "/home/chewi/Projects/flatcar/flatcar-build-scripts/./coreos-overlay-diff.py", line 190, in <module>
    display_difference(args.THEIRS, args.ours, os.path.basename(os.path.abspath(".")), recurse=True)
  File "/home/chewi/Projects/flatcar/flatcar-build-scripts/./coreos-overlay-diff.py", line 109, in display_difference
    commits_we_have = commits_to_pick(src=to_ours, dst=from_theirs)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/chewi/Projects/flatcar/flatcar-build-scripts/./coreos-overlay-diff.py", line 58, in commits_to_pick
    _commits_src_has_without_cherry = cut(grep(git.cherry(dst, src, _bg=False),
                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/sh.py", line 1508, in __call__
    rc = self.__class__.RunningCommandCls(cmd, call_args, stdin, stdout, stderr)
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/sh.py", line 737, in __init__
    self.wait()
  File "/usr/lib/python3.12/site-packages/sh.py", line 799, in wait
    self.handle_command_exit_code(exit_code)
  File "/usr/lib/python3.12/site-packages/sh.py", line 826, in handle_command_exit_code
    raise exc
sh.ErrorReturnCode_2: 

  RAN: /usr/bin/grep '+ 6cd2151eab1e0ff4834b94f250c9a78b0d29ca0e
+ fb79dad6fd97054290620943ea0bcb6b69a98137
+ 7e0a4836796bd5986697a9f27dccaff25f0ba787
+ e3903f11930d9c04f4cf4c047cf2c794aca9ac19
+ 6045acb9720b3bfc4f1f0c77907011e8abf25907
+ 06fa6b4913afbfcc72cccfdee94941aaf0034865
' '^+'

  STDOUT:


  STDERR:
/usr/bin/grep: ^+: No such file or directory

Expected behavior

It outputs information about which commits to pick.

Additional information

I would fix this now, but I imagine it's still used in places that don't have sh 2.x. We could support both, but quite a few lines would need to be duplicated. The syntax is quite different.

[jepio]

@sayanchowdhury / @tormath1 do we use this script?

[tormath1]

Good question. I think most of the package diff is now handled from bash script: https://github.com/flatcar/flatcar-build-scripts/blob/6495dd7bf80b6ec7e5646cf06a50eadb6dea996c/package-diff (see: https://github.com/flatcar/scripts/blob/29f7c259bd77173ae91b4e65cb8dd4ef424d5482/ci-automation/image_changes.sh#L310)

[chewi]

Seems likely, given that I can't see coreos-overlay-diff.py referenced anywhere across the org. @krnowak, please could confirm. If it's redundant, I'll just close this and drop it in #160.

[krnowak]

@chewi: I don't think I have ever used the script. Feel free to kill it.