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

rake eating ARGV #277

Open
mpalmer opened this issue May 24, 2014 · 11 comments
Open

rake eating ARGV #277

mpalmer opened this issue May 24, 2014 · 11 comments

Comments

@mpalmer
Copy link

mpalmer commented May 24, 2014

Somewhere in the last few years, since 41a334b (which came from this mailing list post, I believe), ARGV has once again started getting mangled by option parsing. Since I've recently started avoiding the need to bundle exec rake everywhere (by re-execing rake if it isn't already running inside bundler) I need access to the entire ARGV to pass back to exec.

I haven't attached a patch, because the only solution I've got so far is duping ARGV into a temporary variable before calling init in Rake::Application#run, then restoring ARGV (with ARGV.replace(orig_argv)) afterwards -- and I'm not sure that's particularly kosher. If, on the other hand, you'd be happy to merge such a patch, let me know and I'll put it together and whack in some test cases.

@drbrain
Copy link
Collaborator

drbrain commented Nov 24, 2014

This line is doing it, but has been doing that since 2009 (maybe earlier as that commit moves things around).

This should be easy to fix.

@mpalmer
Copy link
Author

mpalmer commented Nov 25, 2014

Thanks for fixing this, @drbrain. That's a great way to fix it, too -- not using ARGV directly is always a good way to go, I've found.

@drbrain
Copy link
Collaborator

drbrain commented Nov 26, 2014

No problem!

@drbrain
Copy link
Collaborator

drbrain commented Dec 2, 2014

I had to revert this fix as it causes problems such as rails/spring#366 and there doesn't seem to be two ways to do this.

I'm not sure there is an easy alternative that won't involve some manual work by you to re-exec rake.

@drbrain drbrain reopened this Dec 2, 2014
@mpalmer
Copy link
Author

mpalmer commented Dec 2, 2014

Aaaah, bummer. I don't entirely understand the referenced bug report -- it sounds like spring is manually futzing with ARGV itself to inject extra arguments into Rake, which seems like a Bad Idea, overall, but there's no denying that the change to Rake did seem to break other people's stuff, and I think it was the right call to revert this change.

Unfortunately, I can't see any way of getting around this problem without a change in rake. By the time the first line of my code gets executed (in my Rakefile), the damage is already done -- ARGV has been modified, and I'm boned. My original "dodgy hack", of wrapping the call to init in Rake::Application#run with a copy/replace of ARGV does work -- it's what I've been running on my local machine since I opened this bug -- but, as I said originally, I didn't submit it as a PR because I felt it was an excessively ugly way of fixing the problem. Your approach was much better, but appears to violate the expectations of existing consumers of the Rake API.

I can think of a couple of ways forward:

  1. Deprecate the existing way that people are late-passing parameters into Rake.application via ARGV. Provide a new way to pass arguments into Rake.application, possibly as simple as just documenting the argv accessor. Detect when ARGV has changed between when Rake::Application was instantiated and when #run is called (take a private copy of ARGV at the same time as @argv = ARGV, and compare it to ARGV) and emit a deprecation warning, with the intent to remove looking at ARGV at all during #run in Rake 11, some months from now. The logic for selecting whether to use @argv or ARGV for arguments will involve checking the private dup of ARGV made in Rake::Application#initialize.
  2. Move to Rake 11 now, with your previous patch re-applied. Non-backwards-compatible change, etc etc. Brutal, but effective.
  3. Use my dodgy hack of duping then restoring ARGV around the call to #init in #run. Nasty, and will require some comments to explain to future generations what the deuce is happening here, but it'll solve the problem and doesn't require anyone else's ARGV-mangling code to ever need to know anything changed.

If 1 or 3 sound like the best way forward to you, I'm happy to put together a PR for consideration, just let me know and I'll break out the text editor.

@drbrain
Copy link
Collaborator

drbrain commented Dec 2, 2014

It's not necessarily spring by itself, but spring + new relic or something else that calls Rake.application and gets captured.

With Ruby 2.2 coming soon releasing a Rake 11 now is a difficult thing to support.

I prefer something along the lines of option 3 but don't yet have an idea that's easy for future generations to understand. I gave some thought to this before reverting it but wasn't coming up with any good names or any way to explain it without several paragraphs.

@mpalmer
Copy link
Author

mpalmer commented Dec 2, 2014

Well... we might just need several paragraphs then. I'll put together a PR for option 3 and see what I can come up with.

@mpalmer
Copy link
Author

mpalmer commented Dec 2, 2014

Bah, I've just done a bit more digging, and it looks like just passing ARGV.dup) to the call to OptionParser#parse! in Rake::Application#handle_options seems to do the trick quite nicely. Not sure why I didn't dig that deep previously. PR incoming.

@bretweinraub
Copy link

ah i see this thread now.

maybe its my own doing .... but 10.4.2 has really blown up my world because of this [probably how our tool was built; we really depending on knowing which command line args rake cared about via the edited ARGV].

But its a weird thing .... I just seem to get 10.4.2 off a prod box .... its like some kind of sticky. Gem list doesn't show it but ARGV non-eating cannot be removed..... server down. Not your fault really. Don't code to undoc-ed features....

@bretweinraub
Copy link

rvm implode-d # seems to work

mpalmer referenced this issue in ruby/rake Dec 17, 2014
Change the way that OptionParser.parse! is called, so that it doesn't
directly sink its teeth into ARGV.  This necessitated changing the way that
collect_command_line_tasks get the list of command-line arguments to examine
to get tasks and environment variable definitions, but that's a relatively
minor thing.
@mpalmer
Copy link
Author

mpalmer commented Dec 17, 2014

Sorry that my change broke your application, @bretweinraub. Would an instance method on Rake::Application that returned the arguments that Rake didn't "eat" as a result of option parsing satisfy your requirements? The information about what arguments were treated as options and which weren't is still kept within Rake::Application, it just isn't exposed somewhere.

If such an instance method would suit you, let me know and I'll whip up a PR for @drbrain to merge. If that wouldn't suit for some reason, we can discuss what it is you do need, exactly, and I'll come up with a proposed fix one way or another.

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

3 participants