-
Notifications
You must be signed in to change notification settings - Fork 79
[dev] Dependency Guidelines
First, consider if adding the dependency is actually necessary. Adding a dependency carries risk: you have to trust someone else's code. Additionally, if a dependency gets abandoned, it may put us in an awkward position when it breaks with a new Python version.
Some valid reasons for why a dependency might be needed, along with some example dependencies that fit those reasons:
- There is no alternative (
wxPython
) - Any alternative we'd write ourselves would be too slow, since we'd be
writing it in Python (
python-lz4
) - It would be way too much code to write an alternative ourselves
(
chardet
)
If a dependency can't be avoided, consider if it can't be made optional. It's okay for an entire feature to become defunct or absent when an optional dependency is not installed. Some examples:
- If
lxml
is missing, BAIN will not try to validate FOMODs, they will instead all be accepted. - If
PyMuPDF
is missing, the doc browser will not display PDFs and instead show a binary mess for them. - If
send2trash
is missing, recycling a file on Linux or macOS will always permanently delete files.
There are some significant advantages to making dependencies optional:
- Optional dependencies will never force us to remain on an old Python
version. Worst case, we can upgrade the Python version and simply
comment out the dependency in
requirements.txt
. - It means that if a dependency ends up becoming abandoned and incompatible with newer Python versions/other dependencies/etc., the worst that will happen to us is that we may have to retire a feature.
There are some downsides too, however:
- It creates more potential end user environments that we have to support. For that reason, we should always try to keep the number of files we use any particular optional dependency in as low as possible. Ideally, each optional dependency is only used by one file. That way we can much more easily reason about what will happen if the dependency is missing.
There are several places to edit. Let's go through them, with
python-zstandard
as an example.
For the use case, let's imagine Bethesda decide to add another
compression option to their saves for the next game, namely zstd
(along with the already existing uncompressed, lz4
and zlib
options). We want to add support for that.
Note: we will keep this as an optional dependency to
demonstrate how that works, but this use case would actually
justify a hard dependency, since we would actually need this
dependency to parse some files created by the new game. We
made python-lz4
required on the same grounds when we needed
it for Skyrim SE.
We obviously have to add the actual dependency so it will get installed by the CI and developers:
send2trash==1.8.2; sys_platform != 'win32'
# For Nexus Mods integration
websocket-client==1.5.1
+# For reading zstd-compressed saves in Todd's Funtime Adventure Hour
+zstandard==0.21.0
# Compile/Build-time ----------------------------------------------------------
isort==5.12.0
pygit2==1.12.1
Note the fixed version - we fix all versions in our
requirements.txt
and only upgrade them after at least
checking the changelogs of affected dependencies.
This file is needed for complying with third-party license requirements. They almost all state that a copy of the license has to be distributed along with the program (see point 2 on the python-zstandard license in the following diff for an example):
For more information on this, and how to apply and follow the GNU AGPL, see
<http://www.gnu.org/licenses/>.
+====================================================
+::LICENSE:: python-zstandard
+::URL:: https://github.com/indygreg/python-zstandard
+====================================================
+
+Copyright (c) 2016, Gregory Szorc
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or without modification,
+are permitted provided that the following conditions are met:
+
+1. Redistributions of source code must retain the above copyright notice, this
+list of conditions and the following disclaimer.
+
+2. Redistributions in binary form must reproduce the above copyright notice,
+this list of conditions and the following disclaimer in the documentation
+and/or other materials provided with the distribution.
+
+3. Neither the name of the copyright holder nor the names of its contributors
+may be used to endorse or promote products derived from this software without
+specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
+ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR
+ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
=======================================
::LICENSE:: requests
::URL:: https://github.com/psf/requests
See [dev] LICENSE-THIRD-PARTY for more information on this file.
If the dependency has a way to retrieve version info in its
Python files (usually called __version__
, but that's not
guaranteed), then we need to edit dump_environment
:
f'{lz4.library_version_string()}')
except ImportError:
lz4_ver = 'not found'
+ try:
+ import zstandard
+ zstandard_ver = zstandard.__version__
+ except ImportError:
+ zstandard_ver = 'not found (optional)'
try:
import yaml
yaml_ver = yaml.__version__
@@ -319,6 +324,7 @@ def dump_environment(wxver=None):
f' - packaging: {packaging_ver}',
f' - PyMuPDF: {pymupdf_ver}',
f' - python-lz4: {lz4_ver}',
+ f' - python-zstandard: {zstandard_ver}',
f' - PyYAML: {yaml_ver}',
f' - requests: {requests_ver}',
f' - websocket-client: {websocket_client_ver}',
If the dependency does not have anything like that (e.g.
send2trash
does not), just skip this step.
If the dependency is required, you will also want to edit
_import_deps
, so that a nice error message is shown when
the dependency is missing:
@@ -154,6 +154,10 @@ def _import_deps():
import yaml
except ImportError:
deps_msg += u'- PyYAML\n'
+ try:
+ import zstandard # not actually required, just here to demonstrate how to do it
+ except ImportError:
+ deps_msg += ' - python-zstandard\n'
try:
import vdf
except ImportError:
And on that note, if your dependency is required, test to make sure WB will boot and show the nice 'dependency missing' message without it. If it doesn't and you get an ugly tkinter traceback instead, you will probably have to use the same trick that we use for chardet and vdf (see those imports for more details).
Finally, we should add some documentation:
<li><a href="https://pypi.org/project/websocket-client">websocket-client</a>:
Wrye Bash will be able to integrate with Nexus Mods if this is
installed.</li>
+ <li><a href="https://pypi.org/project/zstandard">python-zstandard</a>: Wrye
+ Bash will be able to read and write zstd-compressed saves from Todd's
+ Funtime Adventure Hour if this is installed.</li>
</ul>
<h2 id="bain">Installers Tab <a class="back2top" href="#contents">Back to top</a></h2>
You are now ready to use the dependency. Remember to try-import it if it's optional and handle the case when it's missing!
The easiest way to figure out if upgrades are available is
to open PyCharm's Python Interpreter
settings page. Hit
Ctrl+Alt+S (or open the settings menu in some other
fashion) and navigate to Project: wrye-bash > Python Interpreter
.
Installed dependencies will be shown here, along with an
upwards arrow if they can be upgraded. You can even upgrade
them directly from here, but before you do:
We pin all our dependencies' versions for a reason. So go out there and read the changelogs to see if anything would obviously affect us.
Should be obvious - at the very least, launch WB and check if things still work.
Edit requirements.txt
to pin the dependencies at the
upgraded versions. If you had to fix stuff, commit that as
well.
Dependency upgrades are generally very boring and don't cause problems, so unless they actually caused problems you had to fix, just squash the changes into some other, more interesting commit. But if you did have to fix stuff, it's probably best to keep the commit separate.
Wrye Bash currently follows a sort-of "rolling release" policy for this. If you notice that a new version of a dependency is out, feel free to try upgrading it.