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 .x resolution and --resolve command #130

Merged
merged 5 commits into from
Feb 21, 2018

Conversation

philpennock
Copy link
Contributor

  • New persistent file, ~/.gimme/versions/known-versions.txt, which is
    kept in sorted unique order
  • Add --resolve for handling version specifiers in a
    normally-lightweight manner, where stdout gets the resolved version.
  • Handle NN.x (and NN.MM.x) as a version specifier, against the
    known versions
  • Have the --known output be cached to disk; there's a cache bypass
    mechanism (flag), and a cache age control environment variable.
  • Rework some cache handling stuff to be in cleaner functional
    abstractions
  • Adds a _version_sort filter
  • Bug-fix for stable age using access-time not mod-time on darwin|*bsd

See the README updates for notes on interaction between --resolve and
git tag resolution, and how to avoid that.

Fixes #129
Resolves #110

@philpennock
Copy link
Contributor Author

nb: I actually want to resolve this into master, but since it adds to the documentation added by #128 I've based the review diff off of that.

@philpennock
Copy link
Contributor Author

philpennock commented Feb 17, 2018

Re travis build failures: I disbelieve that ./gimme -h can stall for 10 minutes, as a result of anything in this PR, or at all. I was wrong, I messed up.

* New persistent file, `~/.gimme/versions/known-versions.txt`, which is
  kept in sorted unique order
* Add `--resolve` for handling version specifiers in a
  normally-lightweight manner, where stdout gets the resolved version.
* Handle `NN.x` (and `NN.MM.x`) as a version specifier, against the
  known versions
* Have the `--known` output be cached to disk; there's a cache bypass
  mechanism (flag), and a cache age control environment variable.
* Rework some cache handling stuff to be in cleaner functional
  abstractions
* Adds a `_version_sort` filter
* Bug-fix for stable age using access-time not mod-time on darwin|*bsd

See the README updates for notes on interaction between `--resolve` and
git tag resolution, and how to avoid that.

Fixes #129
Resolves #110
1.1 1.10 1.2 is bad.
1.1 1.2 .. 1.9 1.10 is good.
@philpennock
Copy link
Contributor Author

NB: Do not merge until #131 is approved and merged into this; GitHub handles stacked PRs very badly.

Copy link
Contributor

@AlekSi AlekSi left a comment

Choose a reason for hiding this comment

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

I don't know bash that well, so I mostly checked the documentation.

Thank you for working on this!

@@ -18,6 +18,8 @@
#+ -f --force force - remove the existing go installation if present prior to install
#+ -l --list list - list installed go versions and exit
#+ -k --known known - list known go versions and exit
#+ --force-known-update - when used with --known, ignores the cache and updates
#+ -r --resolve resolve - resolve a version specifier to a version, show that and exit

This comment was marked as spam.

This comment was marked as spam.

README.md Outdated
@@ -164,7 +164,8 @@ versions and point Gimme at that instead.
Invoke `gimme -k` or `gimme --known` to have Gimme report the versions which
can be installed; invoking `gimme stable` installs the version which the Go
Maintainers have declared to be stable. Both of these involve making
non-cached network requests to retrieve this information.
network requests to retrieve this information, although the `--known` output
is cached. (Use `--force-update` to ignore the cache).

This comment was marked as spam.

This comment was marked as spam.

@AlekSi
Copy link
Contributor

AlekSi commented Feb 21, 2018

Does not work in Raspbian:

pi@raspberrypi:~/gimme $ bash --version
GNU bash, version 4.4.12(1)-release (arm-unknown-linux-gnueabihf)
Copyright (C) 2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

pi@raspberrypi:~/gimme $ ./gimme -k
./gimme: line 478: /home/pi/.gimme/versions/known-versions.txt.new: No such file or directory
mv: cannot stat '/home/pi/.gimme/versions/known-versions.txt.new': No such file or directory
cat: /home/pi/.gimme/versions/known-versions.txt: No such file or directory

Here is a trace: https://gist.github.com/AlekSi/13afc22c0d62008118f5633c8341f9a4

@@ -92,6 +103,21 @@ _sha256sum() {
fi
}

# sort versions, handling 1.10 after 1.9, not before 1.2
# FreeBSD sort has --version-sort, none of the others do

This comment was marked as spam.

This comment was marked as spam.

mjs added a commit to jumptrading/influx-spout that referenced this pull request Feb 21, 2018
... and use quotes to avoid undesirable YAML float conversion.

See also:
- travis-ci/gimme#132
- travis-ci/gimme#130
Another issue caught by AlekSi: the code updating the known versions
didn't handle `~/.gimme/versions/` not already existing.
@philpennock
Copy link
Contributor Author

Identified the issues with Raspbian above: it's not Raspbian. It's that ~/.gimme/versions didn't already exist in a place where the new code assumed that it did.

The sort versions are handled: either sort supports --version-sort, or we fall back to --general-numeric-sort after zero-padding any digit sequences after a dot, to make the general-numeric-sort work.

If there's a sort(1) which doesn't handle --general-numeric-sort then that's an issue to care about. But while it's a long-opt and not POSIX, the modern BSD reimplementations of sort make sure to include that flag for compatibility. It's one of the GNUisms which is now "standard" on any maintained platform. For other platforms, we might want to look for a gsort command? But that's a case of "if it's really an issue, fix it".

Copy link
Contributor

@meatballhat meatballhat left a comment

Choose a reason for hiding this comment

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

🎷 🐻

@meatballhat meatballhat merged commit e61f3e6 into doc_versions_policy Feb 21, 2018
@meatballhat meatballhat deleted the dotx_support branch February 21, 2018 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants