Skip to content

WIP: Ensure rails new . --master builds a complete bleeding edge application (not just core gems) #38761

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

Closed
wants to merge 20 commits into from

Conversation

Schwad
Copy link
Contributor

@Schwad Schwad commented Mar 18, 2020

Summary

In PR #38631 I implemented support for the --master option in rails new. This allows quick and easy functionality for devs to test bleeding edge Rails features as soon as they are committed. (Prior to that we had --edge which only pointed to latest stable. If ran from within the master Rails version it would point to master branch, but not otherwise)

There was an ensuing discussion with @matthewd where he pointed out that in the future this could become quite a sharp tool. Say in 2022 someone ran the --master flag from a 6.0 rails version but master was on 6.2 which had a radically different file structure. You could have a situation where you have a file structured like 6.0 but pointing to 6.2 Gemfile.

@rafaelfranca currently has retained the feature for the next release, but we have had a discussion about improving this flag to make it more unique than --edge and --dev.

The pitched idea was to have the flag manually write a bleeding edge Gemfile pointing to master, bundle install it, and then use that to grab the latest rails with bundle exec, but all invoked with the current interface of rails new . --master.

This pull request is a WIP attempt to continue this discussion and pull this idea together. It currently doesn't work, but that's likely more due to my inexperience with building out novel new generators within rails than the quality of the idea.

It is a small PR, and it should be clear what I'm trying to accomplish. I would love feedback especially from the prior discussion participants (including @kaspth ).

I'm also available to pair on this (I have a Tuple license I can invite from) sometime this week or next week.

Thanks much again for the quality discussion around this! ❤️

@Schwad
Copy link
Contributor Author

Schwad commented Mar 18, 2020

Note: Obviously this would also require me to revert the prio --master implementation (which would be redundant with this). I will include that commit if we're able to come to a working consensus here.

@@ -243,6 +249,17 @@ def config_target_version
end
end

class MasterGemfileGenerator < AppBuilder # :nodoc:
def install_shell_gemfile
template "GemfileMasterRailsInstaller", "Gemfile"
Copy link
Contributor

Choose a reason for hiding this comment

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

So we're using this to essentially build a local Rails checkout that can then generate an application?

Not really liking installing this Gemfile that'll linger afterward. We may as well just do a shallow clone via git clone https://github.com/rails/rails.git. We essentially don't need any other lines in the file and if we install the checkout at a known location, we can do /path/to/rails/railties/exe/rails new …

end

def generate_edge_rails_app
run 'bundle exec rails new . --edge'
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you actually pass . and it'll generate into the current directory without naming the app "."? Think we also want to pass on extra options ala --webpack=stimulus.

@@ -0,0 +1,6 @@
source 'https://rubygems.org'
Copy link
Contributor

Choose a reason for hiding this comment

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

Not liking the camelcase naming on the file I gotta say 😄

@@ -243,6 +249,17 @@ def config_target_version
end
end

class MasterGemfileGenerator < AppBuilder # :nodoc:
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the Generator suffix right for this?

end

private

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this.

@options = generator.options
if generator.options[:master]
puts '== Generating new rails app based off of rails/rails master branch =='
MasterGemfileGenerator.new
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, not exactly liking this call structure, but I don't know where else to put this.

@kaspth
Copy link
Contributor

kaspth commented Apr 5, 2020

I'm wondering if we don't want a one file shell script that we can run instead of hooking in multiple places in our generators setup? E.g.

#!/usr/bin sh
set -ex

git clone https://github.com/rails/rails ~/.rails/rails # Or where ever's better. This'll update the clone if it exists, right?

app_name=shift
~/.rails/rails/railties/exe/rails new $app_name --edge $@ # Or however the hell you pass the remaining arguments

@Schwad
Copy link
Contributor Author

Schwad commented Apr 8, 2020

@kaspth thank you for taking a look and your thoughts! 😅

My first few commits addressed some of your feedback, and the last one is a slight peek at possibly going down the shell script route. I actually really like that idea. Unless you change your mind I might nuke my old approach and proceed thusly.

Obviously 'testing' this manually is... interesting.. but at least our friends gave us some test coverage of this command so as long as we pass that we should be in good shape. 😇

@rails-bot
Copy link

rails-bot bot commented Jul 7, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@rails-bot rails-bot bot added the stale label Jul 7, 2020
@rails-bot rails-bot bot closed this Jul 14, 2020
jonathanhefner added a commit to jonathanhefner/rails that referenced this pull request Sep 1, 2020
When running `rails new` with a prerelease flag like `--edge`, the
project's Gemfile will point to the specified prerelease version, but
the project itself is generated using the active version of Rails.  This
can cause problems if, for example, the prerelease version expects a
different file structure than the active version.  It also precludes
using generator options that are new in the prerelease version.

This commit adds a mechanism to install and use prerelease versions of
Rails when running `rails new`.

Closes rails#38761.

Co-authored-by: Nick Schwaderer <nicholas.schwaderer@gmail.com>
jonathanhefner added a commit to jonathanhefner/rails that referenced this pull request Oct 12, 2021
When running `rails new` with a prerelease flag like `--edge`, the
project's Gemfile will point to the specified prerelease version, but
the project itself is generated using the active version of Rails.  This
can cause problems if, for example, the prerelease version expects a
different file structure than the active version.  It also precludes
using generator options that are new in the prerelease version.

This commit adds a mechanism to install and use prerelease versions of
Rails when running `rails new`.

Closes rails#38761.

Co-authored-by: Nick Schwaderer <nicholas.schwaderer@gmail.com>
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.

2 participants