Skip to content

Comments

Use dulwich to fetch global and local git config#5935

Merged
neersighted merged 6 commits intopython-poetry:masterfrom
fredrikaverpil:use-dulwich-to-read-git-config
Jul 13, 2022
Merged

Use dulwich to fetch global and local git config#5935
neersighted merged 6 commits intopython-poetry:masterfrom
fredrikaverpil:use-dulwich-to-read-git-config

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?

@fredrikaverpil fredrikaverpil mentioned this pull request Jul 2, 2022
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.

@github-actions
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