lilydjwg / nvchecker

New version checker for software releases
MIT License
421 stars 68 forks source link

Pain points in end-user migration to nvchecker 2 #161

Closed eli-schwartz closed 3 years ago

eli-schwartz commented 3 years ago

What is the purpose of using json for record files? I track them in git due to $REASONS.

While painfully migrating v2 formats from things that worked (ini) to things that also work, but do it ugly and in twice as many lines (toml) while removing features without clear replacements and resulting in nvchecker-ini2toml producing files that repeatedly crash nvchecker while I try to figure out what is incompatible this time... I realize that running nvchecker and comparing the newver file reached unusually high levels of ugly, and in a manner that does practical damage to my git repo: it uses unsorted, unprettified json, breaking git log/diff, and preventing linewise compression of unchanged values. (The oldver file is likewise unprettified and in random order, but doesn't also shuffle the elements every time.)

I realize this could be solved by using json.dump(dict(sorted(new.items(), indent=2, fp))) to sort and prettify it. But it does not seem like a very valuable change to begin with, so I wonder if it might not be beneficial to revert entirely. I can't figure out what the change in format was supposed to accomplish.

lilydjwg commented 3 years ago

The reason for file format change is to make them standard and interchangeable. People won't wonder if some command line or regex will be received as intended because the format is clearly defined. And people can parse and generate them using any programming languages.

The plain text version record files seem clean, but it continuously has parsing issues (line breaks in version string, spaces in software names).

I've switched to pretty-printing JSON so it looks good to human and git.

The reason for more verbose configuration, specifying source each time, is to make it possible for plugins. While the old format looks cleaner, a hard-coded list of source names is needed, preventing support of source plugins and creating repetitive work when one wants to programmatically understand them.

nvchecker-ini2toml should produce usable TOML files. If not, please file a bug report.

eli-schwartz commented 3 years ago

Missing/renamed options without proper deprecations, stuff like:

Traceback (most recent call last):
  File "/usr/lib/python3.8/site-packages/toml/decoder.py", line 510, in loads
    ret = decoder.load_line(line, currentlevel, multikey,
  File "/usr/lib/python3.8/site-packages/toml/decoder.py", line 777, in load_line
    value, vtype = self.load_value(pair[1], strictly_valid)
  File "/usr/lib/python3.8/site-packages/toml/decoder.py", line 905, in load_value
    raise ValueError("This float doesn't have a leading "
ValueError: This float doesn't have a leading digit

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/bin/nvchecker", line 33, in <module>
    sys.exit(load_entry_point('nvchecker==2.1', 'console_scripts', 'nvchecker')())
  File "/usr/lib/python3.8/site-packages/nvchecker/__main__.py", line 34, in main
    entries, options = core.load_file(
  File "/usr/lib/python3.8/site-packages/nvchecker/core.py", line 188, in load_file
    keymanager = KeyManager(keyfile)
  File "/usr/lib/python3.8/site-packages/nvchecker/util.py", line 47, in __init__
    keys = toml.load(f)['keys']
  File "/usr/lib/python3.8/site-packages/toml/decoder.py", line 156, in load
    return loads(f.read(), _dict, decoder)
  File "/usr/lib/python3.8/site-packages/toml/decoder.py", line 513, in loads
    raise TomlDecodeError(str(err), original, pos)
toml.decoder.TomlDecodeError: This float doesn't have a leading digit (line 2 column 1 char 7)
Error: No module named 'nvchecker_source.vcs'

Is there even a replacement for this? How do I check versions from a git repo (e.g. cgit) if it is "removed without replacement" per the release notes?

The plain text version record files seem clean, but it continuously has parsing issues (line breaks in version string, spaces in software names).

I'm a bit surprised anyone is using versions with line breaks as I've never seen a software released like that, nor a system that supports them, so if there was one, I'd expect it to be removed via regex rather than breaking the record files...

regarding software names, softare might include them, but packaging systems remove them -- so I guess I assumed they would be listed in nvchecker.ini/.toml as software-name. I wouldn't personally be fussed if that were the officially recommended solution. :D

Sorry for being a grump about this... there were just a lot of changes that hit me right in the middle of a holiday and I had no idea what to do to continue checking for things.

I've switched to pretty-printing JSON so it looks good to human and git.

Thank you.

The reason for more verbose configuration, specifying source each time, is to make it possible for plugins. While the old format looks cleaner, a hard-coded list of source names is needed, preventing support of source plugins and creating repetitive work when one wants to programmatically understand them.

I'm not sure I understand this. You cannot iterate over the keys and check if they are known? I assumed that was how key/value pairs used to be handled.

If you must hardcode them, there are other solutions, like having each source plugin register itself and thus be dynamically appended to the list.

lilydjwg commented 3 years ago

toml.decoder.TomlDecodeError: This float doesn't have a leading digit (line 2 column 1 char 7)

What is the relevant part of TOML like? nvchecker-ini2toml shouldn't produce invalid TOMLs.

How do I check versions from a git repo (e.g. cgit) if it is "removed without replacement" per the release notes?

There is a git source.

I'm a bit surprised anyone is using versions with line breaks as I've never seen a software released like that.

It was a regex source config that captured too much.

I wouldn't personally be fussed if that were the officially recommended solution.

The recommendation was to avoid special characters. This wasn't very clear and people has put spaces in the names, which broke the plain text format. And in the ini config I don't exactly know what counts as special.

there were just a lot of changes that hit me right in the middle of a holiday.

I'm sorry about that. You could continue to use the old version until you have time to switch. I tried to smooth the switch.

You cannot iterate over the keys and check if they are known?

With third-party plugins they are unknown. With 2.x people can drop plugin modules in anywhere in Python's sys.path.

like having each source plugin register itself and thus be dynamically appended to the list.

There is no such registration mechanism in Python's importing system. To import something the name must be known. There are ways to work around this but it's clearer to look up the documentation or source code with an explicit source config.

One way is to specify plugins to load in the config. If your config is spread in multiple places (like archlinuxcn/repo does), it would still need to be explicit about which source plugin to load for each package so all needed plugins are registered.

Another way is to make all plugins register in some place, like put a file in a directory --- no, three directories, one for the system packager, one for system admin, and one for the user.

eli-schwartz commented 3 years ago

What is the relevant part of TOML like? nvchecker-ini2toml shouldn't produce invalid TOMLs.

That error came from my recursively included keyfile = "apikey.crypt" which was still in ini form and took much headscratching for me to figure out as it didn't mention the filename it was choking on. It might be a good idea to catch config file errors, and report "config file %s is invalid" before reraising the error.

There is a git source.

Thank you, but I think this should be documented as the preferred migration path... Also consider adding FutureWarning in the previous release for features you intend to remove soon.

...

Registering plugins: https://packaging.python.org/guides/creating-and-discovering-plugins/

nvchecker already uses namespace packages here to good effect.

lilydjwg commented 3 years ago

Thank you. Documentation and error messages have been updated.

I'm surprised that there is a pkgutil module that comes with Python's standard library but completely left out in its documentation for the importing system.

I'll use FutureWarning when possible before removing features. The 2.0 release came out in a bit hurry (because we need batch requests for AUR but it was not possible without a big refactor).