Skip to content

Commit

Permalink
Fix issue with packaging.parse throwing InvalidVersion in the upgrade…
Browse files Browse the repository at this point in the history
…_config() function when trying to parse the informational version string VERSION set by bdbag when it is running in a "frozen" (e.g., with cx_Freeze) environment. In such cases, VERSION is set to something like 1.7.1-frozen, which is not PEP-440 compliant. This was not an issue in previous releases due to the fact that the implementation used pkg_resources.parse_version which was not as strict.

The code in upgrade_config() has been changed to parse the PEP-440 compliant version returned by packaging.parse rather than use the global string VERSION, which can still be (and is) used elsewhere for purely informational and descriptive purposes.

Unfortunately this wasn't caught be unit testing before the release because of the complexity of unit testing pre-built frozen environments. It wasn't until other automation that freezes `bdbag` into a pre-built installable software package generated an installer that was tested manually did it get exposed.
  • Loading branch information
mikedarcy committed Jul 30, 2023
1 parent 4e53037 commit 33043d5
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 6 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
# CHANGE LOG

## 1.7.1
Fix issue with `packaging.parse` throwing `InvalidVersion` in the `upgrade_config()` function when trying to parse the
informational version string `VERSION` set by `bdbag` when it is running in a "frozen" (e.g., with `cx_Freeze`) environment.
In such cases, `VERSION` is set to something like `1.7.1-frozen`, which is not `PEP-440` compliant.
This was not an issue in previous releases due to the fact that the implementation used `pkg_resources.parse_version` which was not as strict.

The code in `upgrade_config()` has been changed to parse the `PEP-440` compliant version returned by `packaging.parse`
rather than use the global string `VERSION`, which can still be (and is) used elsewhere for purely informational and descriptive purposes.

## 1.7.0
* PR: [#54](https://github.com/fair-research/bdbag/pull/54): Add support for passing a local profile path for profile validation. Thanks to [Bernhard Hampel-Waffenthal](https://github.com/prettybits) for the contribution.
* [#40](https://github.com/fair-research/bdbag/issues/40): Replace deprecated use of `pkg_resources` with `importlib-metadata` and `packaging`.
Expand Down
4 changes: 2 additions & 2 deletions bdbag/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

logger = logging.getLogger(__name__)

__version__ = "1.7.0"
__version__ = "1.7.1"
__bagit_version__ = "1.8.1"
__bagit_profile_version__ = "1.3.1"

Expand All @@ -41,7 +41,7 @@
version = distribution("bdbag").version
VERSION = version + '' if not getattr(sys, 'frozen', False) else version + '-frozen'
except PackageNotFoundError: # pragma: no cover
VERSION = __version__ + '-dev' if not getattr(sys, 'frozen', False) else __version__ + '-frozen'
VERSION = __version__ + '.dev'
PROJECT_URL = 'https://github.com/fair-research/bdbag'

try:
Expand Down
13 changes: 9 additions & 4 deletions bdbag/bdbag_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@
import json
from collections import OrderedDict
from packaging.version import parse as parse_version
from importlib_metadata import distribution, PackageNotFoundError
from bdbag import get_typed_exception, safe_move, \
DEFAULT_CONFIG_PATH, BAG_PROFILE_TAG, BDBAG_PROFILE_ID, VERSION
DEFAULT_CONFIG_PATH, BAG_PROFILE_TAG, BDBAG_PROFILE_ID, VERSION, __version__
from bdbag.fetch import Megabyte
from bdbag.fetch.auth.keychain import DEFAULT_KEYCHAIN_FILE, write_keychain

Expand Down Expand Up @@ -132,7 +133,7 @@
}

DEFAULT_CONFIG = {
CONFIG_VERSION_TAG: VERSION,
CONFIG_VERSION_TAG: __version__,
BAG_CONFIG_TAG:
{
BAG_SPEC_VERSION_TAG: DEFAULT_BAG_SPEC_VERSION,
Expand Down Expand Up @@ -215,7 +216,11 @@ def upgrade_config(config_file):
config = json.loads(cf.read(), object_pairs_hook=OrderedDict)

new_config = None
if parse_version(VERSION) > parse_version(config.get(CONFIG_VERSION_TAG, "0")):
try:
version = distribution("bdbag").version
except PackageNotFoundError: # pragma: no cover
version = __version__
if parse_version(version) > parse_version(config.get(CONFIG_VERSION_TAG, "0")):
new_config = DEFAULT_CONFIG.copy()
config_items = [BAG_CONFIG_TAG, FETCH_CONFIG_TAG, RESOLVER_CONFIG_TAG, ID_RESOLVER_TAG]
copy_config_items(config, new_config, config_items)
Expand All @@ -224,7 +229,7 @@ def upgrade_config(config_file):
if updated and new_config:
safe_move(config_file)
write_config(new_config, config_file)
print("Updated configuration file [%s] to current version format: %s" % (config_file, str(VERSION)))
print("Updated configuration file [%s] to current version format: %s" % (config_file, str(version)))


def copy_config_items(old_config, new_config, key_names):
Expand Down

0 comments on commit 33043d5

Please sign in to comment.