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.

Sign up for GitHub

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

Jump to bottom

Expose setting TLS file and dir lookup locations via settings module #884

Merged
merged 4 commits into from
Mar 7, 2019
Merged

Expose setting TLS file and dir lookup locations via settings module #884

merged 4 commits into from
Mar 7, 2019

Conversation

webknjaz
Copy link
Contributor

@webknjaz webknjaz commented Mar 2, 2019 •
edited
Loading

Usage:

import pygit2
pygit2.settings.ssl_cert_file = '/path/to/file'
pygit2.settings.ssl_cert_dir = '/path/to/folder'
del pygit2.settings.ssl_cert_file
pygit2.settings.set_ssl_cert_locations('/path/to/new/file', '/path/to/new/folder')

Co-Authored-By: Sriram Raghu imbuedhope@gmail.com

Closes #876
Superseeds and closes #879

@webknjaz webknjaz marked this pull request as ready for review March 2, 2019 11:29
@webknjaz webknjaz mentioned this pull request Mar 2, 2019
Closed
@webknjaz webknjaz changed the title Refactoring of SSL cert locations options exposure Expose setting TLS file and dir lookup locations via settings module Mar 2, 2019
webknjaz
webknjaz commented Mar 2, 2019
View reviewed changes
pygit2/settings.py Outdated
@property
def ssl_cert_file(self):
"""TLS certificate file path."""
raise AttributeError(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jdavid since we're tracking what we set anyway, we have all values to also expose this for reading. I think it'd be better consistency with this.

Copy link
Member

Choose a reason for hiding this comment

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

Agree.

@imbuedhope
Copy link
Contributor

Might be good to make it clear that Settings should not be instantiated by users too. Especially since it's accessible as pygit2.Settings. With your changes creating a Settings object will cause the ssl certs to reset.

I'm not sure if anyone is using the object, but it's better to play it safe I think. This was not an issue until now since, there was no state was maintained in the Settings class.

@webknjaz
Copy link
Contributor Author

webknjaz commented Mar 2, 2019

Should it be a singleton?

@jdavid
Copy link
Member

jdavid commented Mar 2, 2019

Could you please squash the commits to make it easier to review?

@imbuedhope
Copy link
Contributor

A singleton could work. I'm not sure if there are scenarios where it makes sense to have multiple Settings objects, it doesn't feel like there would be.

@webknjaz
Copy link
Contributor Author

webknjaz commented Mar 5, 2019

@imbuedhope multiple settings may be useful if you deal with multiple repos with different requirements. But for that, it'd have to be thread-safe. Is it now?

@webknjaz webknjaz force-pushed the feature+refactoring/ssl-cert-locations-options branch 3 times, most recently from 32bf19a to 3b16e41 Compare March 5, 2019 12:51
@webknjaz
Copy link
Contributor Author

webknjaz commented Mar 5, 2019

@jdavid rebased, only atomic commits left.

@webknjaz webknjaz force-pushed the feature+refactoring/ssl-cert-locations-options branch 4 times, most recently from 836d1d2 to 4b55d71 Compare March 5, 2019 15:25
@webknjaz
🎨 Wire up TLS cert option interfaces in settings
ca61026 
Usage::

    import pygit2
    pygit2.settings.ssl_cert_file = '/path/to/file'
    pygit2.settings.ssl_cert_dir = '/path/to/folder'
    del pygit2.settings.ssl_cert_file
    pygit2.settings.set_ssl_cert_locations(
        '/path/to/new/file', '/path/to/new/folder',
    )

Co-authored-by: Sriram Raghu <imbuedhope@gmail.com>

Closes libgit2#876
Superseeds and closes libgit2#879
@webknjaz webknjaz force-pushed the feature+refactoring/ssl-cert-locations-options branch from 4b55d71 to ca61026 Compare March 5, 2019 15:32
@imbuedhope
Copy link
Contributor

imbuedhope commented Mar 5, 2019 •
edited
Loading

@webknjaz your current implementation should be safe with the multiprocessing module since memory is not shared. It will probably break with threading, but I think anyone using it with threading should be expecting the settings to be shared.

@webknjaz
Copy link
Contributor Author

webknjaz commented Mar 6, 2019

@imbuedhope that is true. Though, I believe, libgit2 itself supports and is compiled in thread-safe mode: https://github.com/libgit2/pygit2/blob/f483622/travis/build-manylinux1-wheels.sh#L178.

So to summarize: libgit2 is thread-safe, pygit2 probably isn't. So we don't have to care about that right now.

Can we ignore state-management problem in this PR?

@jdavid jdavid merged commit ca61026 into libgit2:master Mar 7, 2019
@jdavid jdavid mentioned this pull request Mar 9, 2019
Closed
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Mar 20, 2019
py-pygit2: update to 0.28.0.
cedf9d2 
Add test target.

0.28.0 (2019-03-19)
-------------------------

- Upgrade to libgit2 0.28
  `#878 <https://github.com/libgit2/pygit2/issues/878>`_

- Add binary wheels for Linux
  `#793 <https://github.com/libgit2/pygit2/issues/793>`_
  `#869 <https://github.com/libgit2/pygit2/pull/869>`_
  `#874 <https://github.com/libgit2/pygit2/pull/874>`_
  `#875 <https://github.com/libgit2/pygit2/pull/875>`_
  `#883 <https://github.com/libgit2/pygit2/pull/883>`_

- New ``pygit2.Mailmap``, see documentation
  `#804 <https://github.com/libgit2/pygit2/pull/804>`_

- New ``Repository.apply(...)`` wraps ``git_apply(..)``
  `#841 <https://github.com/libgit2/pygit2/issues/841>`_
  `#843 <https://github.com/libgit2/pygit2/pull/843>`_

- Now ``Repository.merge_analysis(...)`` accepts an optional reference parameter
  `#888 <https://github.com/libgit2/pygit2/pull/888>`_
  `#891 <https://github.com/libgit2/pygit2/pull/891>`_

- Now ``Repository.add_worktree(...)`` accepts an optional reference parameter
  `#814 <https://github.com/libgit2/pygit2/issues/814>`_
  `#889 <https://github.com/libgit2/pygit2/pull/889>`_

- Now it's possible to set SSL certificate locations
  `#876 <https://github.com/libgit2/pygit2/issues/876>`_
  `#879 <https://github.com/libgit2/pygit2/pull/879>`_
  `#884 <https://github.com/libgit2/pygit2/pull/884>`_
  `#886 <https://github.com/libgit2/pygit2/pull/886>`_

- Test and documentation improvements
  `#873 <https://github.com/libgit2/pygit2/pull/873>`_
  `#887 <https://github.com/libgit2/pygit2/pull/887>`_

Breaking changes:

- Now ``worktree.path`` returns the path to the worktree directory, not to the
  `.git` file within
  `#803 <https://github.com/libgit2/pygit2/issues/803>`_

- Remove undocumented ``worktree.git_path``
  `#803 <https://github.com/libgit2/pygit2/issues/803>`_


0.27.4 (2019-01-19)
-------------------------

- New ``pygit2.LIBGIT2_VER`` tuple
  `#845 <https://github.com/libgit2/pygit2/issues/845>`_
  `#848 <https://github.com/libgit2/pygit2/pull/848>`_

- New objects now support (in)equality comparison and hash
  `#852 <https://github.com/libgit2/pygit2/issues/852>`_
  `#853 <https://github.com/libgit2/pygit2/pull/853>`_

- New references now support (in)equality comparison
  `#860 <https://github.com/libgit2/pygit2/issues/860>`_
  `#862 <https://github.com/libgit2/pygit2/pull/862>`_

- New ``paths`` optional argument in ``Repository.checkout()``
  `#858 <https://github.com/libgit2/pygit2/issues/858>`_
  `#859 <https://github.com/libgit2/pygit2/pull/859>`_

- Fix speed and windows package regression
  `#849 <https://github.com/libgit2/pygit2/issues/849>`_
  `#857 <https://github.com/libgit2/pygit2/issues/857>`_
  `#851 <https://github.com/libgit2/pygit2/pull/851>`_

- Fix deprecation warning
  `#850 <https://github.com/libgit2/pygit2/pull/850>`_

- Documentation fixes
  `#855 <https://github.com/libgit2/pygit2/pull/855>`_

- Add Python classifiers to setup.py
  `#861 <https://github.com/libgit2/pygit2/pull/861>`_

- Speeding up tests in Travis
  `#854 <https://github.com/libgit2/pygit2/pull/854>`_

Breaking changes:

- Remove deprecated `Reference.get_object()`, use `Reference.peel()` instead


0.27.3 (2018-12-15)
-------------------------

- Move to pytest, drop support for Python 3.3 and cffi 0.x
  `#824 <https://github.com/libgit2/pygit2/issues/824>`_
  `#826 <https://github.com/libgit2/pygit2/pull/826>`_
  `#833 <https://github.com/libgit2/pygit2/pull/833>`_
  `#834 <https://github.com/libgit2/pygit2/pull/834>`_

- New support comparing signatures for (in)equality

- New ``Submodule.head_id``
  `#817 <https://github.com/libgit2/pygit2/pull/817>`_

- New ``Remote.prune(...)``
  `#825 <https://github.com/libgit2/pygit2/pull/825>`_

- New ``pygit2.reference_is_valid_name(...)``
  `#827 <https://github.com/libgit2/pygit2/pull/827>`_

- New ``AlreadyExistsError`` and ``InvalidSpecError``
  `#828 <https://github.com/libgit2/pygit2/issues/828>`_
  `#829 <https://github.com/libgit2/pygit2/pull/829>`_

- New ``Reference.raw_name``, ``Reference.raw_shorthand``, ``Tag.raw_name``,
  ``Tag.raw_message`` and ``DiffFile.raw_path``
  `#840 <https://github.com/libgit2/pygit2/pull/840>`_

- Fix decode error in commit messages and signatures
  `#839 <https://github.com/libgit2/pygit2/issues/839>`_

- Fix, raise error in ``Repository.descendant_of(...)`` if commit doesn't exist
  `#822 <https://github.com/libgit2/pygit2/issues/822>`_
  `#842 <https://github.com/libgit2/pygit2/pull/842>`_

- Documentation fixes
  `#821 <https://github.com/libgit2/pygit2/pull/821>`_

Breaking changes:

- Remove undocumented ``Tag._message``, replaced by ``Tag.raw_message``
EnTeQuAk pushed a commit to mozilla/addons-server that referenced this pull request Mar 25, 2019
@pyup-bot @EnTeQuAk
Update pygit2 to 0.28.0 (#10962)
12ef19a 
This PR updates [pygit2](https://pypi.org/project/pygit2) from **0.27.4** to **0.28.0**.



<details>
  <summary>Changelog</summary>
  
  
   ### 0.28.0
   ```
   -------------------------

- Upgrade to libgit2 0.28
  `878 &lt;https://github.com/libgit2/pygit2/issues/878&gt;`_

- Add binary wheels for Linux
  `793 &lt;https://github.com/libgit2/pygit2/issues/793&gt;`_
  `869 &lt;https://github.com/libgit2/pygit2/pull/869&gt;`_
  `874 &lt;https://github.com/libgit2/pygit2/pull/874&gt;`_
  `875 &lt;https://github.com/libgit2/pygit2/pull/875&gt;`_
  `883 &lt;https://github.com/libgit2/pygit2/pull/883&gt;`_

- New ``pygit2.Mailmap``, see documentation
  `804 &lt;https://github.com/libgit2/pygit2/pull/804&gt;`_

- New ``Repository.apply(...)`` wraps ``git_apply(..)``
  `841 &lt;https://github.com/libgit2/pygit2/issues/841&gt;`_
  `843 &lt;https://github.com/libgit2/pygit2/pull/843&gt;`_

- Now ``Repository.merge_analysis(...)`` accepts an optional reference parameter
  `888 &lt;https://github.com/libgit2/pygit2/pull/888&gt;`_
  `891 &lt;https://github.com/libgit2/pygit2/pull/891&gt;`_

- Now ``Repository.add_worktree(...)`` accepts an optional reference parameter
  `814 &lt;https://github.com/libgit2/pygit2/issues/814&gt;`_
  `889 &lt;https://github.com/libgit2/pygit2/pull/889&gt;`_

- Now it&#39;s possible to set SSL certificate locations
  `876 &lt;https://github.com/libgit2/pygit2/issues/876&gt;`_
  `879 &lt;https://github.com/libgit2/pygit2/pull/879&gt;`_
  `884 &lt;https://github.com/libgit2/pygit2/pull/884&gt;`_
  `886 &lt;https://github.com/libgit2/pygit2/pull/886&gt;`_

- Test and documentation improvements
  `873 &lt;https://github.com/libgit2/pygit2/pull/873&gt;`_
  `887 &lt;https://github.com/libgit2/pygit2/pull/887&gt;`_

Breaking changes:

- Now ``worktree.path`` returns the path to the worktree directory, not to the
  `.git` file within
  `803 &lt;https://github.com/libgit2/pygit2/issues/803&gt;`_

- Remove undocumented ``worktree.git_path``
  `803 &lt;https://github.com/libgit2/pygit2/issues/803&gt;`_
   ```
   
  
</details>


 

<details>
  <summary>Links</summary>
  
  - PyPI: https://pypi.org/project/pygit2
  - Changelog: https://pyup.io/changelogs/pygit2/
  - Repo: http://github.com/libgit2/pygit2
</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdavid jdavid jdavid left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow setting GIT_OPT_SET_SSL_CERT_LOCATIONS option in libgit2
3 participants
@webknjaz @imbuedhope @jdavid