buildinspace / peru

a generic package manager, for including other people's code in your projects
MIT License
1.12k stars 69 forks source link

Merging at the top level broken in git 2.36 #227

Closed raineszm closed 2 years ago

raineszm commented 2 years ago

Importing at the top level seems to be broken by newer versions of git.

This can be reproduced by example given in the README

imports:
    # The dircolors file just goes at the root of our project.
    dircolors: ./
    # We're going to merge Pathogen's autoload directory into our own.
    pathogen: .vim/autoload/
    # The Solarized plugin gets its own directory, where Pathogen expects it.
    vim-solarized: .vim/bundle/solarized/

git module dircolors:
    url: https://github.com/seebi/dircolors-solarized
    # Only copy this file. Can be a list of files. Accepts * and ** globs.
    pick: dircolors.ansi-dark

curl module pathogen:
    url: https://codeload.github.com/tpope/vim-pathogen/tar.gz/v2.3
    # Untar the archive after fetching.
    unpack: tar
    # After the unpack, use this subdirectory as the root of the module.
    export: vim-pathogen-2.3/autoload/

git module vim-solarized:
    url: https://github.com/altercation/vim-colors-solarized
    # Fetch this exact commit, instead of master or main.
    rev: 7a7e5c8818d717084730133ed6b84a3ffc9d0447

When running peru sync this errors with

Traceback (most recent call last):
  File "/opt/homebrew/Cellar/peru/1.3.0_1/libexec/lib/python3.10/site-packages/peru/cache.py", line 332, in merge_trees
    await session.merge_tree_into_index(merge_tree, merge_path)
  File "/opt/homebrew/Cellar/peru/1.3.0_1/libexec/lib/python3.10/site-packages/peru/cache.py", line 150, in merge_tree_into_index
    await self.git('read-tree', '-i', '--prefix', prefix_arg, tree)
  File "/opt/homebrew/Cellar/peru/1.3.0_1/libexec/lib/python3.10/site-packages/peru/cache.py", line 77, in git
    raise GitError(command, process.returncode, stdout, stderr)
peru.cache.GitError: git command "['git', '--git-dir=/Users/raineszm/Programming/Sandbox/perutest/.peru/cache/trees', 'read-tree', '-i', '--prefix', '/', 'b26bbc055f30ee89d4997e5b8c104cfce1f57188']" returned error code 128.
stdout:
stderr: fatal: Invalid prefix, prefix cannot start with '/'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/opt/homebrew/bin/peru", line 12, in <module>
    sys.exit(main())
  File "/opt/homebrew/Cellar/peru/1.3.0_1/libexec/lib/python3.10/site-packages/peru/main.py", line 370, in main
    run_task(command_fn(params))
  File "/opt/homebrew/Cellar/peru/1.3.0_1/libexec/lib/python3.10/site-packages/peru/async_helpers.py", line 29, in run_task
    return asyncio.get_event_loop().run_until_complete(coro)
  File "/opt/homebrew/Cellar/python@3.10/3.10.4/Frameworks/Python.framework/Versions/3.10/lib/python3.10/asyncio/base_events.py", line 646, in run_until_complete
    return future.result()
  File "/opt/homebrew/Cellar/peru/1.3.0_1/libexec/lib/python3.10/site-packages/peru/main.py", line 91, in do_sync
    await imports.checkout(params.runtime, params.scope, params.imports,
  File "/opt/homebrew/Cellar/peru/1.3.0_1/libexec/lib/python3.10/site-packages/peru/imports.py", line 11, in checkout
    imports_tree = await get_imports_tree(runtime, scope, imports)
  File "/opt/homebrew/Cellar/peru/1.3.0_1/libexec/lib/python3.10/site-packages/peru/imports.py", line 25, in get_imports_tree
    imports_tree = await merge_imports_tree(runtime.cache, imports,
  File "/opt/homebrew/Cellar/peru/1.3.0_1/libexec/lib/python3.10/site-packages/peru/merge.py", line 27, in merge_imports_tree
    unified_tree = await cache.merge_trees(
  File "/opt/homebrew/Cellar/peru/1.3.0_1/libexec/lib/python3.10/site-packages/peru/cache.py", line 334, in merge_trees
    raise MergeConflictError(e.stdout) from e
peru.cache.MergeConflictError: Merge conflict in import "dircolors" at "./":

The culprit seems to be https://github.com/buildinspace/peru/blob/f16cd2dd443ff210c748f4d021eab3249be52c08/peru/cache.py#L142 interacting with https://github.com/git/git/commit/cc89331ddc92cd89012eaf7937d167b3e0beaecc

oconnor663 commented 2 years ago

Confirmed. As of Git v2.36 (April 17) it looks like we have 25 failing tests. Your diagnosis looks spot on.

A likely fix is going to be just deleting the lines of code that add trailing slashes. A comment in that code dating back to April 2014 suggests that the trailing slash was never really required and was only included to comply with documentation, so hopefully this fix won't break anyone in practice.

Unfortunately this is the second breaking change we've seen from Git within the past year. (Here's the other one.) The "use Git plumbing commands as a low-level library" strategy has worked well for peru for almost a decade, but I'm worried we're going to be seeing more of these issues, and I'm not thrilled about doing a complete rewrite on top of e.g. libgit2.

oconnor663 commented 2 years ago

Fix released as v1.3.1.