postmodern / chruby

Changes the current Ruby
MIT License
2.86k stars 190 forks source link

debian all, and jessie #327

Closed brettrann closed 9 years ago

brettrann commented 9 years ago

Firstly, for all debian, test/setup has:

 79         system_version="$(< /etc/debian_version | awk -F. '{print $1"."$2}')"

That line does not do what is expected, on current wheezy or jessie. It puts an empty string into system_version.

This is probably what that line intends to be:

system_version="$(awk -F. '{print $1"."$2}' < /etc/debian_version)"

But also, it is used to build a url for http://rvm.io/binaries/debian/

which only contains:

6/                                                 03-Aug-2013 09:48       -
6.0/                                               03-Aug-2013 09:48       -
6.0.4/                                             03-Aug-2013 09:48       -
7/                                                 08-Mar-2015 02:35       -
7.0/                                               08-Mar-2015 02:35       -
jessie_sid/   

So 7.8 for current wheezy which the corrected code generates, still breaks. A glance in 6 and 6.0.4 reveals it contains the same files with the same md5. Perhaps there rvm.io should probably be maintaining more comprehensive lists. But you could probably get functional by going with the lead number.

So:

system_version="$(awk -F. '{print $1}' < /etc/debian_version)"

and finally, since they have no 8 there yet for jessie (8.0 is the current name for jessie test, which is the safer alternative to jessie unstable (jessie_sid which is there), setting jessie_sid for 8 with:

 78         system_name="debian"
 79         system_version="$(awk -F. '{print $1}' < /etc/debian_version )"
 80         if [[ $system_version == "8" ]]; then
 81           echo "setting debian jessie 8.0 to jessie_sid because 8.0 didn't exist at the time of writing this"
 82           system_version="jessie_sid"
 83         fi

Works around it for now.

If you have a preferred path forward let me know. Maybe if rvm.io aren't going to create a comprehensive list of version directories one solution could be to build up a list of candidate versions and try them iteratively if there's a 404 failure? 8.0,8,jessie_sid for example.

Finally after it does download i get errors on tests with shunit2 complaining about bell-style but they pass manually so I'm going to ignore that because I don't know what it's on about. i had set bell-style none in my .bashrc but that should have only been done for login shells, or interactive shells. so my problem.

postmodern commented 9 years ago

@mpapis ping

mpapis commented 9 years ago

for Debian only the first number of version is important for binary compatibility, if there is 8.x released it should be enough to link jessie_sid to 8 - @havenwood has full rights to do that.

as for < file | awk - I agree awk < file is better, RVM does a bit different with cat file|awk|head - to keep the file at beginning of the command, see https://github.com/wayneeseguin/rvm/blob/1d3382c10357caac272d7be0d8cd5d19741f3e37/scripts/functions/detect_system#L58:

_system_version="$(\command \cat /etc/debian_version | \command \awk -F. '{print $1}' | head -n 1)"
mpapis commented 9 years ago

unsubscribing, ping me again if you need more details

brettrann commented 9 years ago

Added a comment on postmodern's commit. Still uses point number, not just lead.

brettrann commented 9 years ago

@mpapis is there a usecase for needing the head -n 1? can /etc/debian_version ever contain multiple lines?

_system_version="$(\command \cat /etc/debian_version | \command \awk -F. '{print $1}' | head -n 1)"

if so, that should be added too.

mpapis commented 9 years ago

the head was added because of https://github.com/wayneeseguin/rvm/issues/3174 => it happened once for Centos, it can happen again - for any system

brettrann commented 9 years ago

Sorry about those first three. I was attempting to get rid of them because they reference work's custom jessie/wheezy builds. Not sure how to delete those commits. (reset hard local, push -f, but now they appear against yours oddly...)

The last two: test/chruby_exec_test.sh fix for a zsh error that complains about arguments to local.

test/setup The final system_version line based on mpapis's suggestions. My hack for jessie_sid which you may not want.

https://github.com/brann/chruby/commit/d04990042e3548d86e5c433c5472f94db46a4883 are two Dockerfile tests that can be run, currently manually, to test wheezy and jessie installs.

havenwood commented 9 years ago

@brann 8 -> jessie_sid symlink added.

brettrann commented 9 years ago

That's working now thanks Havenwood.

Created a pull request containing the code changes, without my jessie_sid hack.

The docker test files i've moved into a branch in myfork, because they don't meet your commit guidelines because they are still manual tests. Feel free to grab them.

Closing this ticket.