Enhanced version-bounding logic and related changes

[atreyasha]

Hi @pbrisbin, I will just enumerate the changes in this branch and also ask some questions.

Changes

1. Version bounds have been implemented. Now, the user can input version bounds, for example downgrade vim=8.1 or downgrade "vim>8.1". Supported logical operators are =|<|>|<=|>=.

   The previous functionality where a user could input a - (eg. downgrade vim-8.1) to specify package regex has now been replaced with = (eg. downgrade vim=8.1) as this is IMO more intuitive.

   The proposed logical operators comply with pacman's version bounding standards, which will help us in the next step of handling version-bound dependencies. Furthermore, this change addresses issue Downgrade via version number on command line #82. Package version comparison is done with the aid of vercmp (https://www.archlinux.org/pacman/vercmp.8.html), which is a pacman tool.

   Qn: Restyled is throwing errors on line 267 of downgrade, it is not able to interpret the regex. Any suggestion on what I could do?

2. Tests have been updated to test the version bounding. However, since our Circle CLI checker is built on a Debian-based system, it cannot run the arch-based vercmp and shows a failing status.

   Qn: Is there any way to change the Docker image of our Circle CLI instance to an arch image? I think we could use a public Docker Hub archlinux image, see (https://circleci.com/docs/2.0/executor-types/#public-images-on-docker-hub) and (https://hub.docker.com/_/archlinux?tab=description).

3. Logic for the --arch command-line option has been removed and the documentation has been updated to reflect this. I also removed the test corresponding to finding packages with the i686 architecture since this is now redundant.

4. There was a small bug in the ignore-package behaviour, in that if one executed downgrade vim-8.2.0510-1 (eg. with old syntax), then at the end the package to be ignored will be outputted as vim-8.2.0510.1. I modified this such that only the real package name vim will be sent for ignoring.

5. I updated our Makefile and made some of the locale commands simpler. Also, I modified the locale command to firstly create a new downgrade.pot template and then update all existing locales with this template to reflect line number changes and to comment out redundant chunks (for example the --arch chunks being commented out in this iteration).

   Here, I have another question. I also want to add a Makefile segment which installs all locales to my system so I can check that they work fine. Ideally, I would like this to be part of the Makefile to make it easier for testing/development. Following is the shell-script command based on the AUR package-build:

   #!/bin/bash
   # assume pwd is ./locale/
   
   for po_file in ./*.po; do
       locale="$(basename "$po_file" .po)"
       mkdir -p "/usr/share/locale/$locale/LC_MESSAGES/"
       msgfmt "$po_file" -o "/usr/share/locale/$locale/LC_MESSAGES/downgrade.mo"
   done

   Qn: Any recommendations on how to include this in the Makefile? Would it be okay if I made a separate shell-script with this code labelled eg. locale.sh and executed that via the Makefile?

6. I added additional GitHub badges (see this branch's readme) to reflect GitHub and AUR version numbers, just thought it looks nice. But maybe you can provide a second opinion if this is overkill 😄

Next steps

Let me what you think of these changes. If all is fine, I will update the readme, man-page documentation and changelog with the updates.

[pbrisbin]

Qn: Restyled is throwing errors on line 267 of downgrade, it is not able to interpret the regex. Any suggestion on what I could do?

That seems like a bug in shfmt that you could report. It's a shame to have to workaround this deficiency, but I'd rather not ignore the whole downgrade file (e.g. the whole project) from Restyled, so I think it's worth trying a few things:

One option is to try extracting a variable,

regex='(...|...|...|)'

if [[ "$temp" =~ $regex ]]; then

Another option is to just not have the conditional,

    breakdown=($(sed -r "s/(.*)\b(>=|<=|<|>|=)\b(.*)/\1 \2 \3/g" <<< "$term"))
    term="${breakdown[0]}"
    operator="${breakdown[1]}"
    version="${breakdown[2]}"

Can the sed be written such that, if the pattern doesn't match, you end up with all of term in breakdown[0] and empty breakdown[1,2]? If so, it would function the same way and not need the conditional at all.

Qn: Is there any way to change the Docker image of our Circle CLI instance to an arch image? I think we could use a public Docker Hub archlinux image, see (https://circleci.com/docs/2.0/executor-types/#public-images-on-docker-hub) and (https://hub.docker.com/_/archlinux?tab=description).

Yes, running CI in Arch would be great. Feel free to experiment with whatever Docker image you think may work.

Historically, I would use mocking to fake any Arch-specific commands in tests. But since you are actually testing vercmp, I don't know that mocking it is a great idea. That said, why are we testing how vercmp behaves? Maybe mocking it is a good idea?

Qn: Any recommendations on how to include this in the Makefile? Would it be okay if I made a separate shell-script with this code labelled eg. locale.sh and executed that via the Makefile?

If this routine is useful to have in the project Makefile, that sounds good to me. I can update the PKGBUILD to call the make task too.

I'm not against extracting a separate script when it makes sense, but this routine is relatively small, so I would probably just in-line it:

.PHONY: install.po
install.po:
        for po_file in ./*.po; do \
          locale="$$(basename "$$po_file" .po)" && \
          mkdir -p "/usr/share/locale/$$locale/LC_MESSAGES/" && \
          msgfmt "$po_file" -o "/usr/share/locale/$$locale/LC_MESSAGES/downgrade.mo"; \
        done

[pbrisbin]

OK, this LGTM!

Unfortunately, the PR contains a lot of different distinct changes of varying size, value, and compatibility, and the git history is quite noisy too. I'd like to rebase/rewrite things to roughly the following distinct commits:

1. Minor packaging fixes

   • Fix extra bolding in CHANGELOG
   • Add new batches, punctuation in README
   • Move .PHONY annotations in Makefile
   • Fix for doc/downgrade.8 target in Makefile
   • Add the msgmerge update and commit fixed downgrade.pot
   • Add {install,uninstall.po} targets

2. Fix for TEXTDOMAINDIR, with downgrade.in

   I have some more Makefile nits here. If we get this in its own PR, I'd be happy to just tweak it myself.

3. Remove --arch

   Major version bump.

4. Implement versioning argument

   Minor version bump, if {name}-{version} still works. Major bump if we're saying we only support {name}={version} now.

How is your git-foo? Would you be willing to do the history slicing for that, or do you mind if I did?

[atreyasha]

Unfortunately, the PR contains a lot of different distinct changes of varying size, value, and compatibility, and the git history is quite noisy too. I'd like to rebase/rewrite things to roughly the following distinct commits:

Awesome. Yes, it is a good idea to rebase for different changes.

1. Minor packaging fixes

   • Fix extra bolding in CHANGELOG
   • Add new batches, punctuation in README
   • Move .PHONY annotations in Makefile
   • Fix for doc/downgrade.8 target in Makefile
   • Add the msgmerge update and commit fixed downgrade.pot
   • Add {install,uninstall.po} targets

2. Fix for TEXTDOMAINDIR, with downgrade.in
   I have some more Makefile nits here. If we get this in its own PR, I'd be happy to just tweak it myself.

3. Remove --arch
   Major version bump.

4. Implement versioning argument
   Minor version bump, if {name}-{version} still works. Major bump if we're saying we only support {name}={version} now.

Agree with these. For 4, the changes would only support {name}={version} (or with other operators such as =|<=|>=|<|>) and no longer support {name}-{version}. So it would need to be a major bump.

How is your git-foo? Would you be willing to do the history slicing for that, or do you mind if I did?

Sorry about the mess, I am new to software development (and any standards whatsoever) so I really just do things haphazardly and clean them up at the end. I am not very good with history slicing, so it would perhaps be better if you are more familiar. But I feel bad that you would need to seive through all of the commits.

Is there something I could do to simplify this?

[pbrisbin]

Is there something I could do to simplify this?

Not really no, but it's not a big deal. For reference, I wouldn't sift through the commits themselves, I would actually sift through the diff -- which in this case is a bit easier, IMO.

% git checkout master
% git checkout -b pb/new-branch
% git checkout -p as/version-logic

With -p, Git would show me each chunk of difference at a time and ask me if i should apply it, ignore it, or edit it then apply. I would do this once, keeping the things I want to be part of the first batch. For most chunks it's an easy yes-no, for some files touched by multiple distinct changes (e.g. README or CHANGELOG) it will require some editing, either as part of -p or after. Then commit, then repeat the checkout -p for the next batch, and so on.

If you'd like to try that, feel free. If not, just let me know when this branch has settled and I'll go ahead.

[atreyasha]

Ah I understand. Let me try this out, will probably help me to try this out :)

Should I submit each commit as a separate PR/branch, or just 4 commits into one new PR?

[pbrisbin]

Should I submit each commit as a separate PR/branch, or just 4 commits into one new PR?

That's actually up to you. I would do 4 separate PRs, personally, because then things will get into master quicker, since we'll review and merge each increment as we go.

[pbrisbin]

It can be annoying to do the PR-from-PR workflow though, so 4 commits in one PR is fine with me -- or even force-pushing the updated commits to this PR.

[pbrisbin]

This has been replaced by #114, which was then itself replaced by #115, #116, and #117.