Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: Shopify/bootsnap
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: v1.10.3
Choose a base ref
...
head repository: Shopify/bootsnap
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: v1.11.1
Choose a head ref
  • 15 commits
  • 22 files changed
  • 4 contributors

Commits on Feb 7, 2022

  1. Configuration menu
    Copy the full SHA
    4d78e68 View commit details
    Browse the repository at this point in the history
  2. Drop Ruby 2.3 support.

    Ref: #402
    Ref: https://bugs.ruby-lang.org/issues/10222
    Ref: ruby/ruby@5754f15
    Ref: ruby/ruby@b6d3927
    
    Up to Ruby 2.3, `require` would resolve symlinks, but `require_relative` wouldn't:
    
    ```ruby
    require 'fileutils'
    FileUtils.mkdir_p("realpath")
    File.write("realpath/a.rb", "p :a_loaded")
    File.symlink("realpath", "symlink") rescue nil
    
    $LOAD_PATH.unshift(File.realpath(__dir__) + "/symlink")
    require "a.rb" # load symlink/a.rb in 2.3 and older, load realpath/a.rb on 2.4 and newer
    require_relative "realpath/a.rb" # noop on 2.4+
    ```
    
    This would easily cause double loading issue when `require` and `require_relative`
    were mixed, but was fixed in 2.4 (https://bugs.ruby-lang.org/issues/10222).
    
    The problem is that `Bootsnap` kinda negated this fix, because `realpath()`
    wouldn't be applied to absolute paths:
    
    ```ruby
    require 'fileutils'
    FileUtils.mkdir_p("realpath")
    File.write("realpath/a.rb", "p :a_loaded")
    File.symlink("realpath", "symlink") rescue nil
    
    $LOAD_PATH.unshift(File.realpath(__dir__) + "/symlink")
    require File.expand_path("symlink/a.rb") # load symlink/a.rb in 3.0 and older, load realpath/a.rb on 3.1 and newer
    require_relative "realpath/a.rb" # noop on 3.1+
    ```
    
    And for performance reasons, Bootsnap tried really hard not to call `realpath`,
    as it's a syscall, instead it used `expand_path`, which is entirely in use
    space and doesn't reach to the file system. So if you had a `symlink` in
    `$LOAD_PATH`, `bootcsnap` would perpetuate this bug, which led to the
    addition of #136.
    
    This was ultimately fixed in Ruby 3.1 (https://bugs.ruby-lang.org/issues/17885),
    now `realpath` is applied even on absolute paths.
    
    While `realpath` is indeed expensive, I think the performance impact is ok if
    we only call it for `$LOAD_PATH` members, rather than for all requirable files.
    So if you have X gems, it's going to be more or less X `realpath` calls.
    
    It would stay a problem if a gem actually contained symlinks and used
    `require_relative`, but it's quite the stretch, and with 3.1 now
    handling it, it's not worth keeping such workaround.
    
    See: #402
    byroot committed Feb 7, 2022
    Configuration menu
    Copy the full SHA
    88b8cd4 View commit details
    Browse the repository at this point in the history

Commits on Feb 8, 2022

  1. Merge pull request #403 from Shopify/drop-2.3-support

    Drop Ruby 2.3 support.
    casperisfine authored Feb 8, 2022
    Configuration menu
    Copy the full SHA
    8b1b45f View commit details
    Browse the repository at this point in the history
  2. Get rid of RealPathCache and the Kernel.require_relative decorator

    Ref: #402
    Ref: https://bugs.ruby-lang.org/issues/10222
    Ref: ruby/ruby@5754f15
    Ref: ruby/ruby@b6d3927
    
    Up to Ruby 2.3, `require` would resolve symlinks, but `require_relative` wouldn't:
    
    ```ruby
    require 'fileutils'
    FileUtils.mkdir_p("realpath")
    File.write("realpath/a.rb", "p :a_loaded")
    File.symlink("realpath", "symlink") rescue nil
    
    $LOAD_PATH.unshift(File.realpath(__dir__) + "/symlink")
    require "a.rb" # load symlink/a.rb in 2.3 and older, load realpath/a.rb on 2.4 and newer
    require_relative "realpath/a.rb" # noop on 2.4+
    ```
    
    This would easily cause double loading issue when `require` and `require_relative`
    were mixed, but was fixed in 2.4 (https://bugs.ruby-lang.org/issues/10222).
    
    The problem is that `Bootsnap` kinda negated this fix, because `realpath()`
    wouldn't be applied to absolute paths:
    
    ```ruby
    require 'fileutils'
    FileUtils.mkdir_p("realpath")
    File.write("realpath/a.rb", "p :a_loaded")
    File.symlink("realpath", "symlink") rescue nil
    
    $LOAD_PATH.unshift(File.realpath(__dir__) + "/symlink")
    require File.expand_path("symlink/a.rb") # load symlink/a.rb in 3.0 and older, load realpath/a.rb on 3.1 and newer
    require_relative "realpath/a.rb" # noop on 3.1+
    ```
    
    And for performance reasons, Bootsnap tried really hard not to call `realpath`,
    as it's a syscall, instead it used `expand_path`, which is entirely in use
    space and doesn't reach to the file system. So if you had a `symlink` in
    `$LOAD_PATH`, `bootcsnap` would perpetuate this bug, which led to the
    addition of #136.
    
    This was ultimately fixed in Ruby 3.1 (https://bugs.ruby-lang.org/issues/17885),
    now `realpath` is applied even on absolute paths.
    
    While `realpath` is indeed expensive, I think the performance impact is ok if
    we only call it for `$LOAD_PATH` members, rather than for all requirable files.
    So if you have X gems, it's going to be more or less X `realpath` calls.
    
    It would stay a problem if a gem actually contained symlinks and used
    `require_relative`, but it's quite the stretch, and with 3.1 now
    handling it, it's not worth keeping such workaround.
    
    See: #402
    byroot committed Feb 8, 2022
    Configuration menu
    Copy the full SHA
    fb857fe View commit details
    Browse the repository at this point in the history
  3. Merge pull request #402 from Shopify/skip-realpath-cache-if-useless

    Get rid of RealPathCache and the `Kernel.require_relative` decorator
    casperisfine authored Feb 8, 2022
    Configuration menu
    Copy the full SHA
    5476c07 View commit details
    Browse the repository at this point in the history

Commits on Feb 16, 2022

  1. Fix typo

    The typo was introduced by accident in #399.
    teoljungberg committed Feb 16, 2022
    Configuration menu
    Copy the full SHA
    8f9c7f1 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    18b701a View commit details
    Browse the repository at this point in the history
  3. Bind rb_get_path to better respect Kernel#require duck-type

    Kernel#require turns its argument into a path by calling `to_path` if it
    responds to it, and then into a String (the argument or the return value
    of `to_path`) by using implicit conversion (`to_str`).
    
    Co-authored-by: Étienne Barrié <etienne.barrie@gmail.com>
    byroot and etiennebarrie committed Feb 16, 2022
    Configuration menu
    Copy the full SHA
    671d1d4 View commit details
    Browse the repository at this point in the history
  4. Merge pull request #406 from Shopify/rb-get-path

    Bind `rb_get_path` to better respect Kernel#require duck-type
    casperisfine authored Feb 16, 2022
    Configuration menu
    Copy the full SHA
    cb1adae View commit details
    Browse the repository at this point in the history

Commits on Feb 22, 2022

  1. Configuration menu
    Copy the full SHA
    4f60408 View commit details
    Browse the repository at this point in the history
  2. Drop dependency on FileUtils

    Fix: #404
    
    But also it's annoying to have delayed `require` like this one.
    byroot committed Feb 22, 2022
    Configuration menu
    Copy the full SHA
    843c101 View commit details
    Browse the repository at this point in the history
  3. Merge pull request #407 from Shopify/no-file-utils

    Drop dependency on FileUtils
    casperisfine authored Feb 22, 2022
    Configuration menu
    Copy the full SHA
    ad68ea5 View commit details
    Browse the repository at this point in the history

Commits on Mar 8, 2022

  1. Release 1.11.0

    byroot committed Mar 8, 2022
    Configuration menu
    Copy the full SHA
    c365322 View commit details
    Browse the repository at this point in the history
  2. Fix can't modify frozen Hash error.

    Fix: #411
    
    The load path cache can be mutated, so we need to unfreeze the hash
    when it happens.
    byroot committed Mar 8, 2022
    Configuration menu
    Copy the full SHA
    94e56f9 View commit details
    Browse the repository at this point in the history
  3. Release 1.11.1

    byroot committed Mar 8, 2022
    Configuration menu
    Copy the full SHA
    5bb937a View commit details
    Browse the repository at this point in the history
Loading