flathub-infra / flatpak-external-data-checker

A tool for checking if the external data used in Flatpak manifests is still up to date
GNU General Public License v2.0
115 stars 34 forks source link

main: Handle git failing on invalid fallback email #426

Closed bbhtt closed 4 months ago

bbhtt commented 4 months ago

Updating the manifest from the x-checker flatpak with --update inside the x-checker Flatpak causes the following error:

HEAD is now at b6053d8 test
Author identity unknown

*** Please tell me who you are.

Run

  git config --global user.email "you@example.com"
  git config --global user.name "Your Name"

to set your account's default identity.
Omit --global to set the identity only in this repository.

fatal: unable to auto-detect email address (got 'bbhtt@fedora.(none)')
Traceback (most recent call last):
  File "/app/bin/flatpak-external-data-checker", line 30, in <module>
    main()
  File "/app/lib/fedc/src/main.py", line 504, in main
    outdated_num, errors_num, updated = asyncio.run(run_with_args(args))
                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/asyncio/runners.py", line 190, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/asyncio/base_events.py", line 654, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "/app/lib/fedc/src/main.py", line 475, in run_with_args
    committed_changes = commit_changes(changes)
                        ^^^^^^^^^^^^^^^^^^^^^^^
  File "/app/lib/fedc/src/main.py", line 201, in commit_changes
    check_call(["git", "commit", "-am", message])
  File "/app/lib/fedc/src/main.py", line 128, in check_call
    subprocess.check_call(args)
  File "/usr/lib/python3.11/subprocess.py", line 413, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['git', 'commit', '-am', 'Update PyQt6-6.4.1.tar.gz to 6.4.2']' returned non-zero exit status 128.

This is because git cannot find a configuration for user.name and user.email as the config file might be outside of Flatpak's access for example in (~/.config/git) which is not mounted to XDG_CONFIG_HOME in the sandbox. In that case git tries to fallback to using pwd database name 1 as user.name and hostname or mailname for user.email 2

But the runtime ships neither of those so it generates a bogus .(none) domain 3 and subsequently the git commit subprocess exits with error.

So generate our own email and commit with it on-the-fly to make sure nothing gets written to any git config.

bbhtt commented 4 months ago

In general even outside the Flatpak this will be good to have, as the user may not have a git config anywhere. And at least on Fedora, /etc/hostname is empty by default, and /etc/mailname is Debian specific. So git's fallback does not quite work.

We don't need to handle user.name as git correctly falls back to passwd. Not having that, means there are bigger problems.

bbhtt commented 4 months ago

Giving access to xdg-config/git fixes this issue but exposes a bunch of other faliure modes like not having access to gpg keys, no pinentry running etc.

I can also change this a bit to make it try committing in a "sanitised environment" (not reading any global/system configs) if the first committing fails.

gpg: signing failed: No pinentry

fatal: failed to write commit object
Traceback (most recent call last):
  File "/app/bin/flatpak-external-data-checker", line 30, in <module>
    main()
  File "/app/lib/fedc/src/main.py", line 504, in main
    outdated_num, errors_num, updated = asyncio.run(run_with_args(args))
                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/asyncio/runners.py", line 190, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/asyncio/base_events.py", line 654, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "/app/lib/fedc/src/main.py", line 475, in run_with_args
    committed_changes = commit_changes(changes)
                        ^^^^^^^^^^^^^^^^^^^^^^^
  File "/app/lib/fedc/src/main.py", line 201, in commit_changes
    check_call(["git", "commit", "-am", message])
  File "/app/lib/fedc/src/main.py", line 128, in check_call
    subprocess.check_call(args)
  File "/usr/lib/python3.11/subprocess.py", line 413, in check_call
    raise CalledProcessError(retcode, cmd)
bbhtt commented 4 months ago

I can also change this a bit to make it try committing in a "sanitised environment" (not reading any global/system configs) if the first committing fails.

Exploring this option: https://github.com/flathub-infra/flatpak-external-data-checker/pull/427 This will work even when gpg is broken