Add gemfile flag to exec command #6676

ankitkataria · 2018-08-24T13:11:30Z

What was the end-user problem that led to this PR?

This PR resolves the issue #5924

What was your diagnosis of the problem?

added a gemfile flag in cli.rb
if options[:gemfile] is set I created a SharedHelper method custom_gemfile which used the predefined find_file function to find and set the environment variable with the argument provided

ghost · 2018-08-24T13:11:33Z

Thanks for opening a pull request and helping make Bundler better! Someone from the Bundler team will take a look at your pull request shortly and leave any feedback. Please make sure that your pull request has tests for any changes or added functionality.

We use Travis CI to test and make sure your change works functionally and uses acceptable conventions, you can review the current progress of Travis CI in the PR status window below.

If you have any questions or concerns that you wish to ask, feel free to leave a comment in this PR or join our #bundler channel on Slack.

For more information about contributing to the Bundler project feel free to review our CONTRIBUTING guide

blackgurl1981 · 2018-08-24T13:28:27Z

Hi

colby-swandale · 2018-08-24T13:32:05Z

This needs a test to make sure the feature is behaving as expected and to make sure it will continue to work when new changes are made.

ankitkataria · 2018-08-24T13:36:55Z

@colby-swandale I'll start working on the test. Will update the pr asap. 😉

segiddins · 2018-08-27T04:42:15Z

spec/commands/exec_spec.rb

+
+    bundle "exec --gemfile CustomGemfile test"
+
+    expect(ENV["BUNDLE_GEMFILE"]).to eq("CustomGemfile")


I dont think this will test what you want it to, because the bundle helper actually runs bundler in a subprocess

ankitkataria · 2018-08-27T19:57:42Z

@segiddins Thankyou so much for your comment 😄 I understood what I was doing wrong. I've updated the test now.

colby-swandale

This is pretty much ready to merge

colby-swandale · 2018-08-30T12:31:09Z

lib/bundler/cli/exec.rb

@@ -23,6 +23,10 @@ def initialize(options, args)
    def run
      validate_cmd!
      SharedHelpers.set_bundle_environment
+
+      # setting the gemfile if mentioned


This comment doesn't seem all that useful, the code already explains this

colby-swandale · 2018-08-30T13:08:33Z

Great! Thank you! 🎉

@bundlerbot r+

bundlerbot · 2018-08-30T13:08:34Z

📌 Commit 546d0a5 has been approved by colby-swandale

bundlerbot · 2018-08-30T13:08:45Z

⌛ Testing commit 546d0a5 with merge b8544ed...

Add gemfile flag to exec command ### What was the end-user problem that led to this PR? This PR resolves the issue #5924 ### What was your diagnosis of the problem? - added a `gemfile` flag in `cli.rb` - if `options[:gemfile]` is set I created a SharedHelper method `custom_gemfile` which used the predefined `find_file` function to find and set the environment variable with the argument provided

bundlerbot · 2018-08-30T13:09:39Z

💔 Test failed - status-travis

ankitkataria · 2018-08-30T13:15:25Z

@colby-swandale I don't understand why the jobs corresponding to the failing tests were canceled. What needs to be done? 🤔

colby-swandale · 2018-08-30T13:16:07Z

spec/commands/exec_spec.rb

@@ -6,6 +6,22 @@
    system_gems(system_gems_to_install, :path => :bundle_path)
  end

+  it "works with --gemfile flag" do
+    create_file "CustomGemfile", <<-G


Thinking more about this test, do you need 2 different Gemfiles?

I did that to ensure that the environment variable was set correctly both the times and rackup produced the version corresponding to the mentioned gemfiles.

You're already testing bundle exec with the --gemfile option. So testing it again is redundant, can you please remove it.

colby-swandale · 2018-08-30T13:22:56Z

lib/bundler/shared_helpers.rb

@@ -230,6 +230,11 @@ def write_to_gemfile(gemfile_path, contents)
      filesystem_access(gemfile_path) {|g| File.open(g, "w") {|file| file.puts contents } }
    end

+    def custom_gemfile(gemfile)


I'm a bit confused about this method's name, it's setting an ENV var for the given Gemfile if it exists, so i'm not sure what custom_gemfile means here. This logic also seems pretty particular to bundle exec, does this need to be a method in SharedHelpers?

It was mentioned in the issue that the flag could be extended to other commands as well for consistency. So I made a shared helper which could be used later.
I used "custom" in the sense that it was mentioned by the user. if it is not mentioned then the default gemfiles will be used.

@colby-swandale does this explanation make sense? is there a requirement to change the name of the function? 🤔

You should be able to use the custom Gemfile logic in the CLI

ankitkataria · 2018-09-02T01:19:29Z

Looking at the code in CLI There seemed to be no need of the shared helper. I removed it. Also removed the redundant test. Thankyou for the help @colby-swandale 😄

colby-swandale · 2018-09-03T00:33:19Z

Thanks! @bundlerbot r+

bundlerbot · 2018-09-03T00:33:19Z

📌 Commit 94b7104 has been approved by colby-swandale

bundlerbot · 2018-09-03T00:58:47Z

⌛ Testing commit 94b7104 with merge 78626f6...

Add gemfile flag to exec command ### What was the end-user problem that led to this PR? This PR resolves the issue #5924 ### What was your diagnosis of the problem? - added a `gemfile` flag in `cli.rb` - if `options[:gemfile]` is set I created a SharedHelper method `custom_gemfile` which used the predefined `find_file` function to find and set the environment variable with the argument provided

colby-swandale · 2018-09-03T07:51:36Z

@bundlerbot retry

bundlerbot · 2018-09-03T07:51:49Z

⌛ Testing commit 94b7104 with merge c489248...

Add gemfile flag to exec command ### What was the end-user problem that led to this PR? This PR resolves the issue #5924 ### What was your diagnosis of the problem? - added a `gemfile` flag in `cli.rb` - if `options[:gemfile]` is set I created a SharedHelper method `custom_gemfile` which used the predefined `find_file` function to find and set the environment variable with the argument provided

bundlerbot · 2018-09-03T08:18:40Z

☀️ Test successful - status-travis
Approved by: colby-swandale
Pushing c489248 to master...

Add gemfile flag to exec command ### What was the end-user problem that led to this PR? This PR resolves the issue #5924 ### What was your diagnosis of the problem? - added a `gemfile` flag in `cli.rb` - if `options[:gemfile]` is set I created a SharedHelper method `custom_gemfile` which used the predefined `find_file` function to find and set the environment variable with the argument provided (cherry picked from commit c489248)

Add gemfile flag to exec command

0f8dd3e

Add test for --gemfile flag

663901f

ankitkataria changed the title ~~Add gemfile flag to exec command~~ WIP: Add gemfile flag to exec command Aug 24, 2018

segiddins reviewed Aug 27, 2018

View reviewed changes

Fixes test for --gemfile flag

62a4e05

ankitkataria changed the title ~~WIP: Add gemfile flag to exec command~~ Add gemfile flag to exec command Aug 27, 2018

colby-swandale reviewed Aug 30, 2018

View reviewed changes

colby-swandale added this to the 1.17.0 milestone Aug 30, 2018

Remove comment from exec run

546d0a5

colby-swandale reviewed Aug 30, 2018

View reviewed changes

Uses logic from cli and removes redundant test

94b7104

ankitkataria force-pushed the gemfile-flag branch from e32dc4a to 94b7104 Compare September 2, 2018 01:21

bundlerbot merged commit 94b7104 into rubygems:master Sep 3, 2018

colby-swandale mentioned this pull request Oct 15, 2018

bundle lock should accept most of the options of bundle install #6735

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add gemfile flag to exec command #6676

Add gemfile flag to exec command #6676

ankitkataria commented Aug 24, 2018

ghost commented Aug 24, 2018

blackgurl1981 commented Aug 24, 2018

colby-swandale commented Aug 24, 2018

ankitkataria commented Aug 24, 2018

segiddins Aug 27, 2018

ankitkataria commented Aug 27, 2018

colby-swandale left a comment

colby-swandale Aug 30, 2018

colby-swandale commented Aug 30, 2018

bundlerbot commented Aug 30, 2018

bundlerbot commented Aug 30, 2018

bundlerbot commented Aug 30, 2018

ankitkataria commented Aug 30, 2018

colby-swandale Aug 30, 2018

ankitkataria Aug 30, 2018

colby-swandale Sep 2, 2018

colby-swandale Aug 30, 2018

ankitkataria Aug 30, 2018

ankitkataria Sep 1, 2018

colby-swandale Sep 2, 2018 •

edited

Loading

ankitkataria commented Sep 2, 2018

colby-swandale commented Sep 3, 2018

bundlerbot commented Sep 3, 2018

bundlerbot commented Sep 3, 2018

colby-swandale commented Sep 3, 2018

bundlerbot commented Sep 3, 2018

bundlerbot commented Sep 3, 2018


		bundle "exec --gemfile CustomGemfile test"

		expect(ENV["BUNDLE_GEMFILE"]).to eq("CustomGemfile")

Add gemfile flag to exec command #6676

Add gemfile flag to exec command #6676

Conversation

ankitkataria commented Aug 24, 2018

What was the end-user problem that led to this PR?

What was your diagnosis of the problem?

ghost commented Aug 24, 2018

blackgurl1981 commented Aug 24, 2018

colby-swandale commented Aug 24, 2018

ankitkataria commented Aug 24, 2018

Choose a reason for hiding this comment

ankitkataria commented Aug 27, 2018

colby-swandale left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colby-swandale commented Aug 30, 2018

bundlerbot commented Aug 30, 2018

bundlerbot commented Aug 30, 2018

bundlerbot commented Aug 30, 2018

ankitkataria commented Aug 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colby-swandale Sep 2, 2018 • edited Loading

Choose a reason for hiding this comment

ankitkataria commented Sep 2, 2018

colby-swandale commented Sep 3, 2018

bundlerbot commented Sep 3, 2018

bundlerbot commented Sep 3, 2018

colby-swandale commented Sep 3, 2018

bundlerbot commented Sep 3, 2018

bundlerbot commented Sep 3, 2018

colby-swandale Sep 2, 2018 •

edited

Loading