Robert Eckelmann authored 5 months ago and Luca Weiss committed 5 months ago

In order to make Sphinx be able to list the supported commands, refactor
the argument parsing code to return ArgumentParser object, and align
the function naming closer to how it works in pmbootstrap.

Co-developed-by: Luca Weiss <luca@lucaweiss.eu>

Add some spacing and indent lists correctly.

Pablo Correa Gomez authored 6 months ago and Pablo Correa Gomez committed 5 months ago

GNOME's SSH endpoints areof the form
git@ssh.gitlab.gnome.org:GNOME/Incubator/papers.git and without this
patch, the host could not be found

This also effectively reverts 7d068e20,
because now we actually use the version for something in the program. It
also avoids the problems reported in
postmarketOS/mrhlpr#15 to the
best of my knowledge — I tried reproducing the bug as written in there
and did not get any build failures.

The primary purpose of this version command is for making
experimentation with mypyc easier, but I also can imagine it might be
nice for users to be able to get their version of mrtest easily.

None of the code paths actually make use of the case where it returns
None for anything other than displaying the same error message, if they
display one at all. Some just attempted to use None as if it was an
integer. As such, just display the error in mr.checked_out() to simplify
and deduplicate code.

Mostly for fun. Not sure if there's any real practical benefit. It might
be useful for pmbootstrap later, but adopting it to be compatible with
mypyc would likely be more work, so I'm using this as a playground to
learn how it works.

mypyc doesn't understand the ellipsis in this context:

    mrhlpr/mr.py:177: error: Unsupported statement in class body

And they've functionally the same anyway, so just work around it like
this.

This way we're even stricter with missing types. Let's activate it so we
don't regress.

This should never happen, so we can simplify the function signature by
typing it like this.

frontend.main() only ever returns None, so there's no point in
calling sys.exit with its return value as parameter.

This is useful to test how apk actually will behave when upgrading.

It makes no sense to allow None here given that the first usage of
origin is accessing a property of it with the dot operator, an operation
which would be guaranteed to fail on a None value.

Signed-off-by: Robert Eckelmann <longnoserob@gmail.com>

Pablo Correa Gomez authored 7 months ago and Robert Eckelmann committed 7 months ago

Fixes #21

With more modern packaging methods, the executable flag gets stripped
from the files in data. Adjust mr.py to run the scripts with python3
explicitly, doing it like that is actually better than having these
scripts that are not meant to be executed directly with the executable
flag on the filesystem.

Fixes: issue 20

Pablo Correa Gomez authored 10 months ago and Pablo Correa Gomez committed 10 months ago

Fixes #15

Pablo Correa Gomez authored 10 months ago and Pablo Correa Gomez committed 10 months ago

This is unnecessary, because the code does not use the version. And
setuptools was getting very confused with "mrhlpr.version", as it is
importing the top-level mrhlpr.py file, and failing to import the
module.

Pablo Correa Gomez authored 10 months ago and Newbyte committed 10 months ago

Fixes #17

As it will fail, and that's annoying!

Fixes #18

This should reduce the risk of changes being missed in refactors like
the one discovered by Zach DeCook which was fixed in
7fce8005.

Some are still missing, but it's a start.

mypy cannot find these as they're defined in __init__.py:

$ mypy mrtest.py mrhlpr.py --check-untyped-defs
mrtest/zap_packages.py:36: error: Module has no attribute "is_root_user"  [attr-defined]
mrtest/zap_packages.py:37: error: Module has no attribute "get_sudo"  [attr-defined]
mrtest/add_packages.py:60: error: Module has no attribute "get_virtual_group"  [attr-defined]
mrtest/add_packages.py:63: error: Module has no attribute "is_root_user"  [attr-defined]
mrtest/add_packages.py:64: error: Module has no attribute "get_sudo"  [attr-defined]

This confuses mypy as bytes.decode() returns a str and
subprocess.run(...).stdout returns a bytes object.

While it would be possible to type output as being a Union[bytes, str],
these are really two distinct purposes and I think just renaming the
first use of output to output_raw helps make the code clearer anyway.

Even with the 5 second sleep, it sometimes seems to fail at setting auto
merge, and instead raises GitlabMRClosedError for some reason. As such,
retry up to 3 times with a 5 second sleep in-between, which seems to be
more robust from my testing.

While I understand the intent behind the original message, I always end
up reading it as that providing a thank you comment is discouraged.
While I think the intended meaning is obvious if you think about it, to
me writing it this way feels like a better way to communicate it.

In some cases you may not want to approve an MR but still merge it. For
example, if it's your own MR.

Even if automatic detection of self-merging later is implemented, there
may still be instances where you wish to merge an MR without approving
it, for example if an MR you don't understand well enough to feel
confident merging already has enough approvals for a merge.

Previously you'd just get an error that GITLAB_TOKEN wasn't set in the
environment without any information about how to resolve this.

py3-gitlab is now in community:
https://gitlab.alpinelinux.org/alpine/aports/-/commit/b470cf0780124804e17ce7510418b77640a42b0b

[ci:skip-build]: already built successfully in CI