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

provide a way to query the version #683

Closed
andychu opened this issue Mar 31, 2020 · 6 comments
Closed

provide a way to query the version #683

andychu opened this issue Mar 31, 2020 · 6 comments

Comments

@andychu
Copy link
Contributor

andychu commented Mar 31, 2020

  • Simplest way is $OIL_VERSION => 0.8.pre3
  • Bash also has the awkwardly named BASH_VERSINFO:
$ argv "${BASH_VERSINFO[@]}"
['4', '3', '48', '1', 'release', 'x86_64-pc-linux-gnu']

I guess the reason to have it as an array is to do structured comparisons. That is < and > for version numbers are nontrivial.

But it does make sense to start with a string for POSIX shell compatibility, e.g. for a POSIX shell script that wants to query the version.


Requested on #653 for ble.sh, and I recall other people mentioning it too.

This is still relevant, although we're not being purists ...

https://github.com/oilshell/oil/wiki/Feature-Detection-Is-Better-than-Version-Detection

@akinomyoga
Copy link
Collaborator

If Oil only supports a version string like OIL_VERSION=0.8.pre3, it will be useful that the format of the version string is explicitly defined. One needs to analyze the string within the shell scripts.

@andychu
Copy link
Contributor Author

andychu commented Mar 31, 2020

I'm pretty sure we will stick with x.y.z, so it can be parsed with something like

[[ $OIL_VERSION =~ ([[:alnum:]]+)\.([[:alnum:]]+)\.([[:alnum:]]+) ]]

echo ${BASH_REMATCH[@]}  # although I want a non-bash name for this

(not tested)

If we also have structured data, I think an associative array is better because you can add fields to it in a backward compatible way.

@akinomyoga
Copy link
Collaborator

akinomyoga commented Mar 31, 2020

I think the following points can be documented: Can we assume x and y to be always integers? What is the possible values of z? How to compare zs containing alphabets to know which version is earlier/newer? Or maybe Oil can provide a library function of version comparisons like is-at-least of Zsh.

@andychu
Copy link
Contributor Author

andychu commented Mar 31, 2020

I think the algorithm for is-at-least for Oil would be:

  • Parse into a 4-tuple, where the 3rd element is -1 for "pre" and 0 for empty.
    • So 0.8.pre3 -> (0, 8, -1, 3)
    • 0.8.0 -> (0, 8, 0, 0)
  • Now compare the 4-tuples like Python does. (Maybe it would be nice if Oil can also do this. It's useful for sort(key=...) in Python too, to sort by one column and then another.)

On a related note, I think I may drop the "pre". In other words, right now the numbering scheme is:

  • 0.7.0 -> 0.8.pre1 -> 0.8.pre2 -> ... -> 0.8.pre15 -> 0.8.0

But I discovered that people view "pre" as unstable, when in reality 0.8.pre3 is better than 0.7.0 in basically every way (due to all the tests we run.)

Example of misconception: Homebrew/homebrew-core#51962


So I think the new numbering scheme can just be:

  • 0.8.0 -> 0.8.1 -> 0.8.2 -> ... -> 0.8.15 -> 0.9.0

Which is basically what most projects do. I think I wanted to have a more meaningful concept of 0.7.0 and 0.8.0 but there's not enough user testing occurring for that to happen right now. (Either that or there are not enough Oil bugs, which is unlikely.)

This also has the benefit that is-at-least is simpler -- now it can simply compare 3-tuples rather than 4-tuples.

Since nobody should care about the dozens of 0.{6,7,8}.pre* in 6 months, maybe I will just start changing that, and then we can have a simpler comparison function.


That also gives some more runway before 1.0 :) If we want to maintain the Python style of avoiding 0.10, 1.10 and 2.10.

So basically we will go

  • 0.8.pre1 ... preN -> 0.8.0 (first pass of C++ translation done) -> 0.8.1 -> 0.8.2 ...

@andychu
Copy link
Contributor Author

andychu commented Mar 31, 2020

Well I guess the issue here is that we've never had a patch release. That is we've never had 0.6.1 or 0.7.1, etc.

Python has such patches, but the Linux kernel doesn't I think. I was trying to be more like Python but so far that hasn't been necessary.

@andychu andychu added the stdlib label Mar 31, 2020
@andychu
Copy link
Contributor Author

andychu commented Mar 31, 2020

I should also say that ble.sh and/or #663 are appealing because I would like Oil to change more slowly at some point in the future. That is, it is nice to write the interactive shell in shell/Oil and not Python/C++.

There are likely to be many minor tweaks here and there that can go in a higher level, rather than the core. And that way the versioning of the core will be more stable.

andychu pushed a commit that referenced this issue Apr 8, 2020
ble.sh and neofetch both want to use it.

Addresses issue #683.

Unrelated: Start QSTR doc.
@andychu andychu closed this as completed Apr 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants