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

Use dulwich to fetch global and local git config #5935

Merged
merged 6 commits into from
Jul 13, 2022
Merged

Use dulwich to fetch global and local git config #5935

merged 6 commits into from
Jul 13, 2022

Conversation

fredrikaverpil
Copy link
Contributor

@fredrikaverpil fredrikaverpil commented Jun 30, 2022

Pull Request Check List

Resolves: #5934

  • Added tests for changed code.
  • Fixed broken tests for changed code. 👈 😁
  • Updated documentation for changed code.

Why?

What?

  • Pass the config object into get_transport_and_path .
  • Fix pytest errors following the code change.
  • Bump Dulwich to 0.20.44, so to support URL rewriting using insteadOf/pushInsteadOf.
  • Fix mypy error following the Dulwich bump.

@fredrikaverpil fredrikaverpil marked this pull request as draft June 30, 2022 16:45
@fredrikaverpil fredrikaverpil marked this pull request as ready for review June 30, 2022 18:59
@fredrikaverpil fredrikaverpil marked this pull request as draft June 30, 2022 19:02
@jelmer
Copy link
Contributor

jelmer commented Jun 30, 2022

LGTM from a Dulwich perspective.

@fredrikaverpil fredrikaverpil marked this pull request as ready for review June 30, 2022 21:12
@fredrikaverpil fredrikaverpil changed the title Use dulwich to fetch git config Use dulwich to fetch git local/global config Jun 30, 2022
@fredrikaverpil fredrikaverpil changed the title Use dulwich to fetch git local/global config Use dulwich to fetch global and local git config Jun 30, 2022
@fredrikaverpil
Copy link
Contributor Author

fredrikaverpil commented Jun 30, 2022

@abn you wrote the tests I had to update here, 2 mo ago, so I was wondering if you could take a look at this 😄

Also, I was wondering, shall we also change repo.get_config() here into repo.get_config_stack() as part of this change so it also reads the global git config?

poetry.lock Outdated Show resolved Hide resolved
@fredrikaverpil fredrikaverpil mentioned this pull request Jul 2, 2022
poetry.lock Show resolved Hide resolved
jelmer
jelmer previously approved these changes Jul 5, 2022
Copy link
Contributor

@jelmer jelmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@neersighted neersighted merged commit 879a144 into python-poetry:master Jul 13, 2022
@mkniewallner mkniewallner mentioned this pull request Jul 13, 2022
@guanzo
Copy link

guanzo commented Sep 19, 2022

my gitconfig seems to be crashing poetry install :/

$ cat ~/.gitconfig
[alias]
  c = '!f() { \
    printf '[git commit -m \"%s\"]\n' \"$*\" && \
    git commit -m \"$*\"; \
  }; f'
$ poetry install
Installing dependencies from lock file

Package operations: 3 installs, 0 updates, 0 removals

  • Installing algofi-py-sdk (1.1.1 904a14d)
  • Installing algofi-python-sdk (2.1.2 fe57cc5)
  • Installing tinyman-py-sdk (0.0.6 40ca553)

  IndexError

  bytearray index out of range

  at ~\AppData\Local\Programs\Python\Python310\lib\site-packages\dulwich\config.py:413 in _parse_string
      409│         c = value[i]
      410│         if c == ord(b"\\"):
      411│             i += 1
      412│             try:
    → 413│                 v = _ESCAPE_TABLE[value[i]]
      414│             except IndexError:
      415│                 raise ValueError(
      416│                     "escape character in %r at %d before end of string" % (value, i)
      417│                 )

The following error occurred when trying to handle this error:


  ValueError

  escape character in bytearray(b"\'!f() { \\") at 9 before end of string

  at ~\AppData\Local\Programs\Python\Python310\lib\site-packages\dulwich\config.py:415 in _parse_string
      411│             i += 1
      412│             try:
      413│                 v = _ESCAPE_TABLE[value[i]]
      414│             except IndexError:
    → 415│                 raise ValueError(
      416│                     "escape character in %r at %d before end of string" % (value, i)
      417│                 )
      418│             except KeyError:
      419│                 raise ValueError(

This gitconfig line seems to be the issue c = '!f() { \.

I'm using poetry on Windows and git bash.

@neersighted
Copy link
Member

Please open an issue against Dulwich -- a merged PR is not a good place to report a bug, and the issue lies outside of Poetry's control.

Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Poetry is not using Dulwich to read the global git config (particularly the insteadOf option)
5 participants