Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update actions/checkout action to v3 #9527

Closed
wants to merge 7 commits into from
Closed

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Jan 17, 2023

Problem

The metadata tester is broken. It now emits this every time it runs:

  Traceback (most recent call last):
    File "/usr/local/bin/ckanmetatester", line 11, in <module>
      load_entry_point('CkanMetaTester==0.1', 'console_scripts', 'ckanmetatester')()
    File "/usr/local/lib/python3.7/dist-packages/ckan_meta_tester/__init__.py", line 24, in test_metadata
      environ.get('INPUT_DIFF_META_ROOT'))
    File "/usr/local/lib/python3.7/dist-packages/ckan_meta_tester/ckan_meta_tester.py", line 74, in test_metadata
      for file in self.files_to_test(source):
    File "/usr/local/lib/python3.7/dist-packages/ckan_meta_tester/ckan_meta_tester.py", line 231, in files_to_test
      return self.paths_from_diff(self.branch_diff(Repo('.')))
    File "/usr/local/lib/python3.7/dist-packages/ckan_meta_tester/ckan_meta_tester.py", line 241, in branch_diff
      logging.debug('Looking for merge base between %s and %s', start_ref, repo.head.commit.hexsha)
    File "/usr/local/lib/python3.7/dist-packages/git/refs/symbolic.py", line 226, in _get_commit
      obj = self._get_object()
    File "/usr/local/lib/python3.7/dist-packages/git/refs/symbolic.py", line 219, in _get_object
      return Object.new_from_sha(self.repo, hex_to_bin(self.dereference_recursive(self.repo, self.path)))
    File "/usr/local/lib/python3.7/dist-packages/git/objects/base.py", line 94, in new_from_sha
      oinfo = repo.odb.info(sha1)
    File "/usr/local/lib/python3.7/dist-packages/git/db.py", line 40, in info
      hexsha, typename, size = self._git.get_object_header(bin_to_hex(binsha))
    File "/usr/local/lib/python3.7/dist-packages/git/cmd.py", line [13](https://github.com/KSP-CKAN/NetKAN/actions/runs/3895070850/jobs/6678636081#step:6:14)83, in get_object_header
      return self.__get_object_header(cmd, ref)
    File "/usr/local/lib/python3.7/dist-packages/git/cmd.py", line 1370, in __get_object_header
      return self._parse_object_header(cmd.stdout.readline())
    File "/usr/local/lib/python3.7/dist-packages/git/cmd.py", line 13[29](https://github.com/KSP-CKAN/NetKAN/actions/runs/3895070850/jobs/6678636081#step:6:30), in _parse_object_header
      raise ValueError("SHA could not be resolved, git returned: %r" % (header_line.strip()))
  ValueError: SHA could not be resolved, git returned: b''

Causes (speculative)

  1. git added a "safe directories" feature in mid-2022, which causes it to print an error without even parsing the repo config if the current user doesn't own a repo, to prevent other users from forcing the current user to run code. Adding KSP 1.12.5 to the embedded build map probably updated the docker image to use a newer version of git including these changes.
  2. Presumably such an ownership discrepancy exists somewhere in the labyrinthine gnarled bowels of the GitHub workflow runtime
  3. GitPython doesn't handle this error intelligently, see Cannot retrieve commits from a specific repository gitpython-developers/GitPython#1016; it just returns an empty string somewhere internally, confusing itself

I have not personally loaded up the current docker image and simulated running it in order to probe the details of exactly what's happening in the filesystem. But I find this a plausible explanation for the problem, and if it doesn't work out, we can continue investigating and try something else.

Changes

Now we upgrade from v2 to v3 of actions/checkout, which ignores "safe directories" by default (see actions/checkout#762 and actions/checkout#770), and should mitigate this if that's the cause.

See KSP-CKAN/xKAN-meta_testing#89, which this doesn't seem to actually fix.

@HebaruSan
Copy link
Member Author

HebaruSan commented Jan 17, 2023

Nope, still happens. Delightful.

Ahh, the /usr/bin/git config --global --add safe.directory /home/runner/work/NetKAN/NetKAN command was already present in the v2 runs. Hmm.

@HebaruSan
Copy link
Member Author

git --version output: git version 2.39.0
git rev-parse HEAD output: b74473bae858f23592a92c6403b46a463007de30

@HebaruSan
Copy link
Member Author

But the git version and global config can be different inside the docker image, right?

@HebaruSan
Copy link
Member Author

We can try adding git config --global --add safe.directory '*' to the Dockerfile, but that would require committing to the CKAN repo's master branch, and I'm not sure how I feel about doing that for a mere guess...

@HebaruSan
Copy link
Member Author

HebaruSan commented Jan 18, 2023

$ docker run -it --entrypoint bash kspckan/metadata
root@51cf9f68ad0d:/# git --version
git version 2.20.1

I think that version is from 2018, so it probably doesn't have the safe directory feature.

... but I don't have a repo created by actions/checkout for testing.

@HebaruSan
Copy link
Member Author

Finally got the error to happen locally in my own NetKAN repo! The GitHub workflow prints the entire docker command that it runs, which is very helpful:

$ docker run --workdir /github/workspace -v $(pwd):/github/workspace -e INPUT_SOURCE=commits kspckan/metadata
Traceback (most recent call last):
  File "/usr/local/bin/ckanmetatester", line 11, in <module>
    load_entry_point('CkanMetaTester==0.1', 'console_scripts', 'ckanmetatester')()
  File "/usr/local/lib/python3.7/dist-packages/ckan_meta_tester/__init__.py", line 24, in test_metadata
    environ.get('INPUT_DIFF_META_ROOT'))
  File "/usr/local/lib/python3.7/dist-packages/ckan_meta_tester/ckan_meta_tester.py", line 74, in test_metadata
    for file in self.files_to_test(source):
  File "/usr/local/lib/python3.7/dist-packages/ckan_meta_tester/ckan_meta_tester.py", line 231, in files_to_test
    return self.paths_from_diff(self.branch_diff(Repo('.')))
  File "/usr/local/lib/python3.7/dist-packages/ckan_meta_tester/ckan_meta_tester.py", line 241, in branch_diff
    logging.debug('Looking for merge base between %s and %s', start_ref, repo.head.commit.hexsha)
  File "/usr/local/lib/python3.7/dist-packages/git/refs/symbolic.py", line 226, in _get_commit
    obj = self._get_object()
  File "/usr/local/lib/python3.7/dist-packages/git/refs/symbolic.py", line 219, in _get_object
    return Object.new_from_sha(self.repo, hex_to_bin(self.dereference_recursive(self.repo, self.path)))
  File "/usr/local/lib/python3.7/dist-packages/git/objects/base.py", line 94, in new_from_sha
    oinfo = repo.odb.info(sha1)
  File "/usr/local/lib/python3.7/dist-packages/git/db.py", line 40, in info
    hexsha, typename, size = self._git.get_object_header(bin_to_hex(binsha))
  File "/usr/local/lib/python3.7/dist-packages/git/cmd.py", line 1383, in get_object_header
    return self.__get_object_header(cmd, ref)
  File "/usr/local/lib/python3.7/dist-packages/git/cmd.py", line 1370, in __get_object_header
    return self._parse_object_header(cmd.stdout.readline())
  File "/usr/local/lib/python3.7/dist-packages/git/cmd.py", line 1329, in _parse_object_header
    raise ValueError("SHA could not be resolved, git returned: %r" % (header_line.strip()))
ValueError: SHA could not be resolved, git returned: b''

@HebaruSan
Copy link
Member Author

HebaruSan commented Jan 18, 2023

$ docker run -it --workdir /github/workspace -v $(pwd):/github/workspace -e INPUT_SOURCE=commits --entrypoint bash kspckan/metadata
root@937525a4ec61:/github/workspace# git rev-parse HEAD
fatal: detected dubious ownership in repository at '/github/workspace'
To add an exception for this directory, call:

        git config --global --add safe.directory /github/workspace

!!!!!

So I guess the safe directory feature is a little older than mid-2022; at least back to 2018!

@HebaruSan
Copy link
Member Author

Your branch is really old. Try rebasing on current master.

@HebaruSan
Copy link
Member Author

Validation succeeded after the changes in KSP-CKAN/xKAN-meta_testing#90. 🎉
This one isn't needed anymore.

@HebaruSan HebaruSan closed this Feb 11, 2023
@HebaruSan HebaruSan deleted the fix/inflate-workflow branch February 11, 2023 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants