Address reviewer feedback: check XDG mode properly and make lazy

Co-authored-by: MikeMcQuaid <125011+MikeMcQuaid@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot] 2025-09-09 12:50:51 +00:00
parent 3a358905ac
commit 040fd9cd91
2 changed files with 36 additions and 14 deletions

View File

@ -24,16 +24,22 @@ module Homebrew
else
raise "'HOMEBREW_BUNDLE_FILE' cannot be specified with '--global'" if env_bundle_file.present?
if user_config_home
xdg_path = "#{user_config_home}/Brewfile"
legacy_path = Bundle.exchange_uid_if_needed! { "#{Dir.home}/.Brewfile" }
# Determine if we're in XDG mode by checking if user_config_home is XDG-based
# In bin/brew: XDG_CONFIG_HOME set -> HOMEBREW_USER_CONFIG_HOME="${XDG_CONFIG_HOME}/homebrew"
# XDG_CONFIG_HOME not set -> HOMEBREW_USER_CONFIG_HOME="${HOME}/.homebrew"
using_xdg = user_config_home&.end_with?("/homebrew")
# Prefer existing file to maintain backwards compatibility
if File.exist?(xdg_path) || !File.exist?(legacy_path)
xdg_path
if using_xdg && user_config_home
# XDG mode: check both XDG and legacy locations, preferring existing files
xdg_brewfile = "#{user_config_home}/Brewfile"
if File.exist?(xdg_brewfile)
xdg_brewfile
else
legacy_path
legacy_brewfile = Bundle.exchange_uid_if_needed! { "#{Dir.home}/.Brewfile" }
File.exist?(legacy_brewfile) ? legacy_brewfile : xdg_brewfile
end
elsif user_config_home && File.exist?("#{user_config_home}/Brewfile")
"#{user_config_home}/Brewfile"
else
Bundle.exchange_uid_if_needed! do
"#{Dir.home}/.Brewfile"

View File

@ -26,8 +26,11 @@ RSpec.describe Homebrew::Bundle::Brewfile do
allow(ENV).to receive(:fetch).with("HOMEBREW_USER_CONFIG_HOME", any_args)
.and_return(env_user_config_home_value)
allow(ENV).to receive(:[]).with("XDG_CONFIG_HOME").and_return(nil)
allow(File).to receive(:exist?).with("/Users/username/.homebrew/Brewfile")
.and_return(config_dir_brewfile_exist)
# Allow any other File.exist? calls to return false by default
allow(File).to receive(:exist?).and_return(false)
end
context "when `file` is specified with a relative path" do
@ -175,7 +178,9 @@ RSpec.describe Homebrew::Bundle::Brewfile do
context "when HOMEBREW_USER_CONFIG_HOME is set but neither Brewfile exists" do
let(:config_dir_brewfile_exist) { false }
context "when XDG_CONFIG_HOME is set" do
before do
allow(ENV).to receive(:[]).with("XDG_CONFIG_HOME").and_return("/custom/xdg")
# Mock that both XDG and legacy Brewfiles don't exist
allow(File).to receive(:exist?).with("/Users/username/.homebrew/Brewfile").and_return(false)
allow(File).to receive(:exist?).with("#{Dir.home}/.Brewfile").and_return(false)
@ -185,6 +190,17 @@ RSpec.describe Homebrew::Bundle::Brewfile do
expect(path).to eq(Pathname.new("#{env_user_config_home_value}/Brewfile"))
end
end
context "when XDG_CONFIG_HOME is not set" do
before do
allow(ENV).to receive(:[]).with("XDG_CONFIG_HOME").and_return(nil)
end
it "returns the legacy path" do
expect(path).to eq(Pathname.new("#{Dir.home}/.Brewfile"))
end
end
end
end
context "when HOMEBREW_BUNDLE_FILE has a value" do