Unset ruby envs in openstudio_cli.rb #4068

tijcolem · 2020-09-04T20:21:51Z

Fixes OpenStudio CLI and ruby envs #4067

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

Labels:

If change to an IDD file, add the label IDDChange
If breaking existing API, add the label APIChange
If deemed ready, add label Pull Request - Ready for CI so that CI builds your PR

Review Checklist

This will not be exhaustively relevant to every PR.

Perform a Code Review on GitHub
Code Style, strip trailing whitespace, etc.
All related changes have been implemented: model changes, model tests, FT changes, FT tests, VersionTranslation, OS App
Labeling is ok
If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
If feature, test running new feature, try creative ways to break it
CI status: all green or justified

…to cli

macumber · 2020-09-05T16:39:07Z

This is related to https://github.com/NREL/openstudio-extension-gem/blob/develop/lib/openstudio/extension/runner.rb#L183

Be careful here because there are a lot of possible use cases here (e.g calling the CLI to do standards sizing run in workflow using a bundle, etc)

tijcolem · 2020-09-10T16:11:46Z

Thanks for the heads up @macumber. My understanding with the recent changes of how bundler works as of version > 3.0.1, openstudio_cli.rb should only explicitly set these ruby envs based on arguments to the CLI (e.g. --gem_home). It should never use the parent process envs by doing something like:

export GEM_HOME = /some/path/to/gem_home
export GEM_PATH = /some/path/to/gem_home
# run CLI
openstudio some_command

As far as openstudio-gems, I looked into these and did a simple grep -r GEM_HOME ./openstudio-gems/ to see what might be using them. Besides bundler, these gems make use of them but they also unset them on the init/setup functions.

https://github.com/NREL/openstudio-extension-gem/blob/develop/lib/openstudio/extension/runner.rb#L183-L204

https://github.com/NREL/openstudio-standards/blob/master/lib/openstudio-standards.rb#L416-L427

The only other thing I can think of is ruby bindings but that gets set up by the wrapper - https://github.com/NREL/OpenStudio/blob/develop/ruby/openstudio.rb and we are not modifying those envs there.

I think these changes are what we want and just unset these to prevent unwanted usage in the CLI unless you know of a use case that might cause issues? I can't think of any of looking through things.

macumber · 2020-09-11T02:46:43Z

Are these still accepted as command line arguments?

        o.on('--verbose', 'Print the full log to STDOUT')
        o.on('-I', '--include DIR', 'Add additional directory to add to front of Ruby $LOAD_PATH (may be used more than once)')
        o.on('-e', '--execute CMD', 'Execute one line of script (may be used more than once). Returns after executing commands.')
        o.on('--gem_path DIR', 'Add additional directory to add to front of GEM_PATH environment variable (may be used more than once)')
        o.on('--gem_home DIR', 'Set GEM_HOME environment variable')
        o.on('--bundle GEMFILE', 'Use bundler for GEMFILE')
        o.on('--bundle_path BUNDLE_PATH', 'Use bundler installed gems in BUNDLE_PATH')
        o.on('--bundle_without WITHOUT_GROUPS', 'Space separated list of groups for bundler to exclude in WITHOUT_GROUPS.  Surround multiple groups with quotes like "test development".')

https://github.com/NREL/OpenStudio/blob/develop/src/cli/openstudio_cli.rb#L387

I think this is used in URBANopt: https://github.com/urbanopt/urbanopt-scenario-gem/blob/c4570c9fcf6676d4ff8a17b94a9adf950d515fbd/lib/urbanopt/scenario/scenario_runner_osw.rb#L89

It gets passed in to the extension runner: https://github.com/NREL/openstudio-extension-gem/blob/develop/lib/openstudio/extension/runner.rb#L88

macumber · 2020-09-11T02:52:00Z

The biggest use case to me was to be able to use different versions of the OpenStudio Standards Gem. URBANopt was built around the idea that each project would have a Gemfile that pulls in all the URBANopt gems you want for your project.

tijcolem · 2020-09-11T14:06:11Z

Yep, those are all still excepted args to the cli so no changes there. So you can still do this if you wanted to use a custom openstudio-standards gem.

1 ) Create simple Gemfile for custom openstudio-standards

cat <<EOF >> Gemfile
source 'http://rubygems.org'
ruby "~> 2.5.0"
gem 'openstudio-standards', '= 0.2.12.rc2'
EOF

2 ) Install gems via bundler
bundle install --path custom-standards-gems

3 ) Run cli with custom gems
openstudio --bundle Gemfile --bundle_path custom-standard-gems somefile.rb

Note that the above workflow is unique to OpenStudio versions > 3.0.1 as you now only need to install the gems you want to use with OpenStudio vs all gems.

I don't think we want this anymore: https://github.com/NREL/OpenStudio/blob/develop/src/cli/openstudio_cli.rb#L416-L418 Thanks for pointing it out. The idea here is to drive the CLI with args only and not accept ruby envs from the shell since many users have rvm which sets these automatically and causes unwanted behavior in the CLI.

macumber · 2020-09-12T00:25:52Z

Sounds good Tim!

…e cli

jmarrec · 2020-09-18T14:09:29Z

Building develop, without this fix:

(py38)julien@OS-build-release$ gem list | grep minitest
(py38)julien@OS-build-release$ ctest -j 16 -R Calendar_Test-daylight_saving
Test project /home/julien/Software/Others/OS-build-release
    Start 2825: RubyTest-Calendar_Test-daylight_savings
    Start 2917: CLITest-Calendar_Test-daylight_savings
1/2 Test #2825: RubyTest-Calendar_Test-daylight_savings ...***Failed    0.31 sec
gem install minitest2/2 Test #2917: CLITest-Calendar_Test-daylight_savings ....   Passed    5.19 sec

50% tests passed, 1 tests failed out of 2

Total Test time (real) =   8.00 sec

The following tests FAILED:
	2825 - RubyTest-Calendar_Test-daylight_savings (Failed)
Errors while running CTest
(py38)julien@OS-build-release$ gem install minitest
Fetching: minitest-5.14.2.gem (100%)
Successfully installed minitest-5.14.2
1 gem installed
(py38)julien@OS-build-release$ ctest -j 16 -R Calendar_Test-daylight_saving
Test project /home/julien/Software/Others/OS-build-release
    Start 2825: RubyTest-Calendar_Test-daylight_savings
    Start 2917: CLITest-Calendar_Test-daylight_savings
1/2 Test #2825: RubyTest-Calendar_Test-daylight_savings ...   Passed    0.34 sec
2/2 Test #2917: CLITest-Calendar_Test-daylight_savings ....***Failed    0.48 sec

50% tests passed, 1 tests failed out of 2

Total Test time (real) =   3.29 sec

The following tests FAILED:
	2917 - CLITest-Calendar_Test-daylight_savings (Failed)
Errors while running CTest

Building this PR

(py38)julien@OS-build-release$ gem list | grep minitest
16:minitest (5.14.2)
(py38)julien@OS-build-release$ ctest -j 16 -R Calendar_Test-daylight_saving
Test project /home/julien/Software/Others/OS-build-release
    Start 2917: CLITest-Calendar_Test-daylight_savings
    Start 2825: RubyTest-Calendar_Test-daylight_savings
1/2 Test #2825: RubyTest-Calendar_Test-daylight_savings ...   Passed    0.35 sec
2/2 Test #2917: CLITest-Calendar_Test-daylight_savings ....   Passed    5.15 sec

100% tests passed, 0 tests failed out of 2

Total Test time (real) =   7.97 sec

@tijcolem Works for me! thanks for the fix.

jmarrec · 2020-09-18T14:43:50Z

EDIT: FALSE ALARM

It was picking up my system ruby's bundler at 1.1.16.

rvm use system
gem update --system

now I have 2.1.4 on my system ruby and the test runs just fine.

Original:
It's broken as is:


(py38)julien@OpenStudio-resources (output-control-files *=)$ gem list | grep bundler
bundler (2.1.4, default: 1.16.6)

(py38)julien@OpenStudio-resources (output-control-files *=)$ $os_build_rel/Products/openstudio model_tests.rb -n test_model_articulation1_bundle_no_git_osw

# Running tests with run options :

Running for OpenStudio 3.1.0-alpha+e454bfaffb
Running for OpenStudio 3.1.0-alpha+e454bfaffb (Previous=3.0.1)

# Running tests with run options -n test_model_articulation1_bundle_no_git_osw --seed 26988:

Fetching gem metadata from http://rubygems.org/........
Resolving dependencies...
Using bundler 1.16.6
Fetching openstudio-standards 0.2.11
Installing openstudio-standards 0.2.11
Fetching openstudio-workflow 2.0.0
Installing openstudio-workflow 2.0.0
Bundle complete! 2 Gemfile dependencies, 3 gems now installed.
Bundled gems are installed into `./gems`
Fetching gem metadata from http://rubygems.org/........
Writing lockfile to /home/julien/Software/Others/OpenStudio-resources/gemfiles/bundle_no_git/Gemfile.lock
E

Finished tests in 41.785416s, 0.0239 tests/s, 0.0479 assertions/s.


Error:
ModelTests#test_model_articulation1_bundle_no_git_osw:
RuntimeError: Exit code 1:
Error executing argv: ["--bundle", "/home/julien/Software/Others/OpenStudio-resources/gemfiles/bundle_no_git/Gemfile", "--bundle_path", "/home/julien/Software/Others/OpenStudio-resources/gemfiles/bundle_no_git/gems", "run", "-w", "/home/julien/Software/Others/OpenStudio-resources/testruns/model_articulation1_bundle_no_git.osw/in.osw"]
Error: Unable to resolve dependency: user requested 'bundler (~> 2.1.0)' in :/ruby/2.5.0/rubygems/resolver.rb:231:in `search_for'
:/ruby/2.5.0/rubygems/resolver.rb:283:in `block in sort_dependencies'
:/ruby/2.5.0/rubygems/resolver.rb:277:in `each'
:/ruby/2.5.0/rubygems/resolver.rb:277:in `sort_by'
:/ruby/2.5.0/rubygems/resolver.rb:277:in `with_index'
:/ruby/2.5.0/rubygems/resolver.rb:277:in `sort_dependencies'
:/ruby/2.5.0/rubygems/resolver/molinillo/lib/molinillo/delegates/specification_provider.rb:52:in `block in sort_dependencies'
:/ruby/2.5.0/rubygems/resolver/molinillo/lib/molinillo/delegates/specification_provider.rb:69:in `with_no_such_dependency_error_handling'
:/ruby/2.5.0/rubygems/resolver/molinillo/lib/molinillo/delegates/specification_provider.rb:51:in `sort_dependencies'
:/ruby/2.5.0/rubygems/resolver/molinillo/lib/molinillo/resolution.rb:165:in `initial_state'
:/ruby/2.5.0/rubygems/resolver/molinillo/lib/molinillo/resolution.rb:106:in `start_resolution'
:/ruby/2.5.0/rubygems/resolver/molinillo/lib/molinillo/resolution.rb:64:in `resolve'
:/ruby/2.5.0/rubygems/resolver/molinillo/lib/molinillo/resolver.rb:42:in `resolve'
:/ruby/2.5.0/rubygems/resolver.rb:188:in `resolve'
:/openstudio_cli.rb:515:in `parse_main_args'
:/openstudio_cli.rb:720:in `execute'
:/openstudio_cli.rb:1750:in `<main>'
eval:175:in `eval'
eval:175:in `require_embedded_absolute'
eval:160:in `block in require_embedded'
eval:154:in `each'
eval:154:in `require_embedded'
eval:113:in `require'
eval:3:in `<main>'

    /home/julien/Software/Others/OpenStudio-resources/test_helpers.rb:352:in `block in run_command'
    :/ruby/2.5.0/open3.rb:205:in `popen_run'
    :/ruby/2.5.0/open3.rb:95:in `popen3'
    /home/julien/Software/Others/OpenStudio-resources/test_helpers.rb:336:in `run_command'
    /home/julien/Software/Others/OpenStudio-resources/test_helpers.rb:705:in `sim_test'
    /home/julien/Software/Others/OpenStudio-resources/model_tests.rb:1052:in `test_model_articulation1_bundle_no_git_osw'

1 tests, 2 assertions, 0 failures, 1 errors, 0 skips

jmarrec · 2020-09-18T14:50:17Z

OpenStudio/src/cli/openstudio_cli.rb

Lines 372 to 374 in d21ad25

    
           if ENV['GEM_PATH'] 
        
             new_path << ENV['GEM_PATH'].to_s 
        
           end

will never be entered

jmarrec

Tested that everything works fine for me and that it does fix the problem

…e external ENVs into the cli anymore

ci-commercialbuildings · 2020-09-18T17:04:40Z

CI Results for c71989c:

Linux:
Darwin:
Windows:
- OpenStudio-3.1.0-alpha+1f5567d7ab-Windows.exe
- OpenStudio-3.1.0-alpha+1f5567d7ab-Windows.tar.gz

Unset all ruby envs as these should be set by respective args passed …

53c3869

…to cli

tijcolem added the Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. label Sep 4, 2020

tijcolem self-assigned this Sep 11, 2020

tijcolem added component - CLI severity - Minor Bug labels Sep 11, 2020

tijcolem added 2 commits September 15, 2020 11:46

remove ruby ENV bundle variables as these should be set by args to th…

57be7e7

…e cli

Merge remote-tracking branch 'origin/develop' into issue_4067

d21ad25

tijcolem requested a review from jmarrec September 16, 2020 18:11

Merge remote-tracking branch 'origin/develop' into issue_4067

9493061

jmarrec approved these changes Sep 18, 2020

View reviewed changes

Remove using ENV['GEM_PATH'] when new_path set as we don't want to us…

c71989c

…e external ENVs into the cli anymore

tijcolem merged commit 023ff90 into develop Sep 21, 2020

tijcolem deleted the issue_4067 branch September 21, 2020 15:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unset ruby envs in openstudio_cli.rb #4068

Unset ruby envs in openstudio_cli.rb #4068

tijcolem commented Sep 4, 2020

macumber commented Sep 5, 2020

tijcolem commented Sep 10, 2020

macumber commented Sep 11, 2020 •

edited

Loading

macumber commented Sep 11, 2020

tijcolem commented Sep 11, 2020

macumber commented Sep 12, 2020

jmarrec commented Sep 18, 2020 •

edited

Loading

jmarrec commented Sep 18, 2020 •

edited

Loading

jmarrec commented Sep 18, 2020

jmarrec left a comment

ci-commercialbuildings commented Sep 18, 2020 •

edited

Loading

Unset ruby envs in openstudio_cli.rb #4068

Unset ruby envs in openstudio_cli.rb #4068

Conversation

tijcolem commented Sep 4, 2020

Review Checklist

macumber commented Sep 5, 2020

tijcolem commented Sep 10, 2020

macumber commented Sep 11, 2020 • edited Loading

macumber commented Sep 11, 2020

tijcolem commented Sep 11, 2020

macumber commented Sep 12, 2020

jmarrec commented Sep 18, 2020 • edited Loading

Building develop, without this fix:

Building this PR

jmarrec commented Sep 18, 2020 • edited Loading

EDIT: FALSE ALARM

jmarrec commented Sep 18, 2020

jmarrec left a comment

Choose a reason for hiding this comment

ci-commercialbuildings commented Sep 18, 2020 • edited Loading

macumber commented Sep 11, 2020 •

edited

Loading

jmarrec commented Sep 18, 2020 •

edited

Loading

jmarrec commented Sep 18, 2020 •

edited

Loading

ci-commercialbuildings commented Sep 18, 2020 •

edited

Loading