Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add semver comparrison #201

Merged
merged 1 commit into from
Apr 3, 2024
Merged

Add semver comparrison #201

merged 1 commit into from
Apr 3, 2024

Conversation

marcelldls
Copy link
Contributor

Thoughts @gilesknap?

@marcelldls marcelldls requested a review from gilesknap April 2, 2024 15:37
@marcelldls
Copy link
Contributor Author

Usage:

(.venv) [esq51579@pc0146 ibek]$ ibek version below 2.2.1 && echo Continue
SUCCESS: IBEK 2.0.2.dev0+g928f52e2.d20240402 less than 2.2.1
Continue
(.venv) [esq51579@pc0146 ibek]$ ibek version above 2.2.1 && echo Continue
╭─────────────────────────────── Traceback (most recent call last) ────────────────────────────────╮
│ /home/esq51579/WIP/pr/ibek/src/ibek/version_cmds/commands.py:40 in above                         │
│                                                                                                  │
│   37 │   if Version.coerce(__version__) > Version.coerce(in_ver):                                │
│   38 │   │   print(f"SUCCESS: IBEK {__version__} greater than {in_ver}")                         │
│   39 │   else:                                                                                   │
│ ❱ 40 │   │   raise Exception(f"IBEK {__version__} not greater than {in_ver}")                    │
│   41                                                                                             │
│   42                                                                                             │
│   43 @version_cli.command()                                                                      │
│                                                                                                  │
│ ╭───── locals ─────╮                                                                             │
│ │ in_ver = '2.2.1' │                                                                             │
│ ╰──────────────────╯                                                                             │
╰──────────────────────────────────────────────────────────────────────────────────────────────────╯
Exception: IBEK 2.0.2.dev0+g928f52e2.d20240402 not greater than 2.2.1

Copy link
Member

@gilesknap gilesknap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good so far.

  • I don't think we want an exception. I would like the result to be silent and just return an exit code of 1 if the version match fails and 0 if it succeeds. This makes it easy to use from a bash script (see example in next point)

  • I would like to also be able to compare two strings so i could do something like:

    if ! ibek version below "$BASE" "7.0.8" ; then
       # do something relevant to epics base 7.0.8 and above
    fi

    You could use the logic that single arg means compare with my own version or have different function names for comparing. Or maybe an option to declare that you are comparing against ibeks own version.

  • In the above example script I used what I believe to be the most common scenario. Doing something if a version is greater or equal than a specific point at which a feature was implemented. I had to negate 'below' to do that - should we consider a different scheme where 'greater that or equal' and 'less than or equal' are the two operators??

@marcelldls
Copy link
Contributor Author

Hows something like:

(.venv) [esq51579@pc0146 ibek]$ if ibek compare 1.1.1 "<" 1.1.2; then echo continue; fi
continue
(.venv) [esq51579@pc0146 ibek]$ if ibek compare 1.1.1 ">" 1.1.2; then echo continue; fi

@GDYendell
Copy link
Member

GDYendell commented Apr 3, 2024

Have you considered copying pip's requirements specifier syntax rather than "below", "above", etc.?

https://pip.pypa.io/en/stable/reference/requirement-specifiers/

You could have a command that takes a package and a specifier that covers all cases, e.g. ibek ... $BASE >=7.0.8 or ibek ... $BASE <7.0.8

I think checking ibek itself might be better as a separate command.

@marcelldls
Copy link
Contributor Author

marcelldls commented Apr 3, 2024

Have you considered copying pip's requirements specifier syntax rather than "below", "above", etc.?

https://pip.pypa.io/en/stable/reference/requirement-specifiers/

You could have a command that takes a package and a specifier that covers all cases, e.g. ibek ... $BASE >=7.0.8 or ibek ... $BASE <7.0.8

I think checking ibek itself might be better as a separate command.

Thanks @GDYendell. Was just to get something simple out quickly to discuss really.

Your proposal makes sense. Because of "Use quotes around specifiers in the shell when using >, <, or when using environment markers" I believe your proposal would actually be ibek ... $BASE ">=7.0.8". But fine with that @gilesknap

Agree "checking ibek itself might be better as a separate command"

@marcelldls
Copy link
Contributor Author

@gilesknap Thoughts?
Usage

(.venv) [esq51579@pc0146 ibek]$ if ibek compare "1.1.3" ">1.1.2"; then echo continue; fi
continue
(.venv) [esq51579@pc0146 ibek]$ if ibek compare "1.1.3" "<1.1.2"; then echo continue; fi

@gilesknap
Copy link
Member

Yes very happy with that - now the question is how to do ibek itself.
One way would be:

if ibek compare $(ibek --version) ">2.0.0"; then echo continue; fi

Too ugly? @GDYendell?

@gilesknap
Copy link
Member

gilesknap commented Apr 3, 2024

BTW I assume with this syntax you will support >= >= == too ?

Or maybe all of these ? https://pip.pypa.io/en/stable/reference/requirement-specifiers/#examples

@marcelldls
Copy link
Contributor Author

BTW I assume with this syntax you will support >= >= == too ?

Or maybe all of these ? https://pip.pypa.io/en/stable/reference/requirement-specifiers/#examples

Right now I have

ops = {
">=": operator.ge,
"<=": operator.le,
"==": operator.eq,
">": operator.gt,
"<": operator.lt,
}

@GDYendell
Copy link
Member

if ibek compare $(ibek --version) ">2.0.0"; then echo continue; fi

Too ugly? @GDYendell?

I think this is OK. I like it more than ibek compare implicitly checking ibek if you only give it the version specifier.

@gilesknap
Copy link
Member

Like it !

@marcelldls marcelldls marked this pull request as ready for review April 3, 2024 14:50
Add compare command

Add pip type compare command

Linting

Add fail test
@gilesknap gilesknap merged commit 4505f85 into main Apr 3, 2024
12 checks passed
@marcelldls marcelldls deleted the semver-compare branch April 3, 2024 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants