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

Merging at the top level broken in git 2.36 #227

Closed
raineszm opened this issue May 25, 2022 · 2 comments
Closed

Merging at the top level broken in git 2.36 #227

raineszm opened this issue May 25, 2022 · 2 comments

Comments

@raineszm
Copy link

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

# The git docs say that a --prefix value must end in a slash. That
interacting with git/git@cc89331

@oconnor663
Copy link
Member

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 added a commit that referenced this issue May 25, 2022
As of Git 2.36, this leads to a fatal error when the prefix is empty
(the root of the repo). See #227.
oconnor663 added a commit that referenced this issue May 25, 2022
Changes since 1.3.0:
- BUGFIX: Fixed "invalid prefix" errors in common cases, including in
  the README example project. The immediate cause of the errors was that
  Git 2.36 changed the requirements of the `git read-tree --prefix`
  flag. See #227. Reported by @raineszm.
- As of Git 2.34, handling of `.gitignore` files in the sync directory
  has changed. This is an upstream change, rather than a peru change.
  Previously, peru would emit an error if a sync would overwrite any
  file that wasn't created in a previous sync, even if that file was
  ignored in `.gitignore`. Now, ignoring a file in `.gitignore` will
  suppress this error.
- "curl" modules now include a user-agent header in its requests, to fix
  errors from some hosts that require this header. Contributed by
  @colindean.
- Fix deprecation warnings from Python 3.10.
@oconnor663
Copy link
Member

Fix released as v1.3.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants