From 2f18fff9992d9f53a1cda4c4108d7c08a9d77e9c Mon Sep 17 00:00:00 2001 From: feu Date: Wed, 25 Jul 2018 16:42:16 -0300 Subject: [PATCH 01/10] adjusting svn commands to consider the @ symbol --- Library/Homebrew/download_strategy.rb | 33 +++++++++++++++---- .../Homebrew/unpack_strategy/subversion.rb | 4 ++- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index c63210bc98..eecd2797af 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -131,7 +131,7 @@ class VCSDownloadStrategy < AbstractDownloadStrategy end def cached_location - @clone + @clone end delegate head?: :version @@ -554,18 +554,37 @@ class SubversionDownloadStrategy < VCSDownloadStrategy end def source_modified_time - xml = REXML::Document.new(Utils.popen_read("svn", "info", "--xml", cached_location.to_s)) + xml = REXML::Document.new(Utils.popen_read("svn", "info", "--xml", svn_cached_location)) Time.parse REXML::XPath.first(xml, "//date/text()").to_s end def last_commit - Utils.popen_read("svn", "info", "--show-item", "revision", cached_location.to_s).strip + Utils.popen_read("svn", "info", "--show-item", "revision", svn_cached_location).strip end private - def repo_url - Utils.popen_read("svn", "info", cached_location.to_s).strip[/^URL: (.+)$/, 1] + def escape(svn_url) + # subversion uses '@' to point to a specific revision + # so when the path contains a @, it requires an additional @ at the end + # but this is not consistent through all commands + # the commands are affected as follows: + # svn checkout url1 foo@a # properly checks out url1 to foo@a + # svn switch url2 foo@a # properly switchs foo@a to url2 + # svn update foo@a@ # properly updates foo@a + # svn info foo@a@ # properly obtains info on foo@a + # svn export foo@a@ newdir # properly export foo@a contents to newdir + result = svn_url.to_s.dup + result << "@" if result.include? "@" + result + end + + def svn_cached_location + escape(cached_location) + end + + def repo_url + Utils.popen_read("svn", "info", svn_cached_location).strip[/^URL: (.+)$/, 1] end def externals @@ -582,7 +601,9 @@ class SubversionDownloadStrategy < VCSDownloadStrategy svncommand = target.directory? ? "up" : "checkout" args = ["svn", svncommand] args << url unless target.directory? - args << target + target_arg = target.to_s + target_arg = escape(target_arg) if svncommand == "up" + args << target_arg if revision ohai "Checking out #{@ref}" args << "-r" << revision diff --git a/Library/Homebrew/unpack_strategy/subversion.rb b/Library/Homebrew/unpack_strategy/subversion.rb index 02599f01e2..90c81eb552 100644 --- a/Library/Homebrew/unpack_strategy/subversion.rb +++ b/Library/Homebrew/unpack_strategy/subversion.rb @@ -9,7 +9,9 @@ module UnpackStrategy private def extract_to_dir(unpack_dir, basename:, verbose:) - system_command! "svn", args: ["export", "--force", path, unpack_dir] + path_export = path.to_s + path_export << "@" if path_export.include? "@" + system_command! "svn", args: ["export", "--force", path_export, unpack_dir] end end end From c3516cf4726c6c40b7a2ab7ce9e130e6dfb3bdfc Mon Sep 17 00:00:00 2001 From: feu Date: Wed, 25 Jul 2018 16:52:21 -0300 Subject: [PATCH 02/10] removing unintentional added spaces --- Library/Homebrew/download_strategy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index eecd2797af..a096187e2c 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -131,7 +131,7 @@ class VCSDownloadStrategy < AbstractDownloadStrategy end def cached_location - @clone + @clone end delegate head?: :version From 3098a900d1157acba8df1a4ae23503cbd84d2574 Mon Sep 17 00:00:00 2001 From: feu Date: Wed, 25 Jul 2018 16:59:57 -0300 Subject: [PATCH 03/10] removing unintentional added spaces 2 --- Library/Homebrew/download_strategy.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index a096187e2c..5d441199b5 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -571,8 +571,8 @@ class SubversionDownloadStrategy < VCSDownloadStrategy # the commands are affected as follows: # svn checkout url1 foo@a # properly checks out url1 to foo@a # svn switch url2 foo@a # properly switchs foo@a to url2 - # svn update foo@a@ # properly updates foo@a - # svn info foo@a@ # properly obtains info on foo@a + # svn update foo@a@ # properly updates foo@a + # svn info foo@a@ # properly obtains info on foo@a # svn export foo@a@ newdir # properly export foo@a contents to newdir result = svn_url.to_s.dup result << "@" if result.include? "@" @@ -583,7 +583,7 @@ class SubversionDownloadStrategy < VCSDownloadStrategy escape(cached_location) end - def repo_url + def repo_url Utils.popen_read("svn", "info", svn_cached_location).strip[/^URL: (.+)$/, 1] end From 9594a56879e88d3072eda791288771d86b5a5aa4 Mon Sep 17 00:00:00 2001 From: feu Date: Wed, 25 Jul 2018 19:32:36 -0300 Subject: [PATCH 04/10] removing svn_cached_location and using svn_escape(path) inside subversion.rb --- Library/Homebrew/download_strategy.rb | 39 ++++++------------- .../Homebrew/unpack_strategy/subversion.rb | 18 ++++++++- 2 files changed, 27 insertions(+), 30 deletions(-) diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index 5d441199b5..141a81bbef 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -554,37 +554,18 @@ class SubversionDownloadStrategy < VCSDownloadStrategy end def source_modified_time - xml = REXML::Document.new(Utils.popen_read("svn", "info", "--xml", svn_cached_location)) + xml = REXML::Document.new(Utils.popen_read("svn", "info", "--xml", svn_escape(cached_location))) Time.parse REXML::XPath.first(xml, "//date/text()").to_s end def last_commit - Utils.popen_read("svn", "info", "--show-item", "revision", svn_cached_location).strip + Utils.popen_read("svn", "info", "--show-item", "revision", svn_escape(cached_location)).strip end private - def escape(svn_url) - # subversion uses '@' to point to a specific revision - # so when the path contains a @, it requires an additional @ at the end - # but this is not consistent through all commands - # the commands are affected as follows: - # svn checkout url1 foo@a # properly checks out url1 to foo@a - # svn switch url2 foo@a # properly switchs foo@a to url2 - # svn update foo@a@ # properly updates foo@a - # svn info foo@a@ # properly obtains info on foo@a - # svn export foo@a@ newdir # properly export foo@a contents to newdir - result = svn_url.to_s.dup - result << "@" if result.include? "@" - result - end - - def svn_cached_location - escape(cached_location) - end - def repo_url - Utils.popen_read("svn", "info", svn_cached_location).strip[/^URL: (.+)$/, 1] + Utils.popen_read("svn", "info", svn_escape(cached_location)).strip[/^URL: (.+)$/, 1] end def externals @@ -595,15 +576,17 @@ class SubversionDownloadStrategy < VCSDownloadStrategy end def fetch_repo(target, url, revision = nil, ignore_externals = false) - # Use "svn up" when the repository already exists locally. + # Use "svn update" when the repository already exists locally. # This saves on bandwidth and will have a similar effect to verifying the # cache as it will make any changes to get the right revision. - svncommand = target.directory? ? "up" : "checkout" + svncommand = target.directory? ? "update" : "checkout" args = ["svn", svncommand] - args << url unless target.directory? - target_arg = target.to_s - target_arg = escape(target_arg) if svncommand == "up" - args << target_arg + if svncommand == "checkout" + args << url + args << target + elsif svncommand == "update" + args << svn_escape(target.to_s) + end if revision ohai "Checking out #{@ref}" args << "-r" << revision diff --git a/Library/Homebrew/unpack_strategy/subversion.rb b/Library/Homebrew/unpack_strategy/subversion.rb index 90c81eb552..e9a0c66c5f 100644 --- a/Library/Homebrew/unpack_strategy/subversion.rb +++ b/Library/Homebrew/unpack_strategy/subversion.rb @@ -1,5 +1,20 @@ require_relative "directory" +def svn_escape(svn_path) + # subversion uses '@' to point to a specific revision + # so when the path contains a @, it requires an additional @ at the end + # but this is not consistent through all commands + # the commands are affected as follows: + # svn checkout url1 foo@a # properly checks out url1 to foo@a + # svn switch url2 foo@a # properly switchs foo@a to url2 + # svn update foo@a@ # properly updates foo@a + # svn info foo@a@ # properly obtains info on foo@a + # svn export foo@a@ newdir # properly export foo@a contents to newdir + result = svn_path.to_s.dup + result << "@" if result.include? "@" + result +end + module UnpackStrategy class Subversion < Directory def self.can_extract?(path:, magic_number:) @@ -9,8 +24,7 @@ module UnpackStrategy private def extract_to_dir(unpack_dir, basename:, verbose:) - path_export = path.to_s - path_export << "@" if path_export.include? "@" + path_export = svn_escape(path.to_s) system_command! "svn", args: ["export", "--force", path_export, unpack_dir] end end From a0f30934aa49ca204098e8a0f72e8dac7a241e71 Mon Sep 17 00:00:00 2001 From: feu Date: Wed, 25 Jul 2018 19:46:35 -0300 Subject: [PATCH 05/10] removing unnecessary .to_s --- Library/Homebrew/download_strategy.rb | 2 +- Library/Homebrew/unpack_strategy/subversion.rb | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index 141a81bbef..a7e87cb4ed 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -585,7 +585,7 @@ class SubversionDownloadStrategy < VCSDownloadStrategy args << url args << target elsif svncommand == "update" - args << svn_escape(target.to_s) + args << svn_escape(target) end if revision ohai "Checking out #{@ref}" diff --git a/Library/Homebrew/unpack_strategy/subversion.rb b/Library/Homebrew/unpack_strategy/subversion.rb index e9a0c66c5f..b185d2ae57 100644 --- a/Library/Homebrew/unpack_strategy/subversion.rb +++ b/Library/Homebrew/unpack_strategy/subversion.rb @@ -24,8 +24,7 @@ module UnpackStrategy private def extract_to_dir(unpack_dir, basename:, verbose:) - path_export = svn_escape(path.to_s) - system_command! "svn", args: ["export", "--force", path_export, unpack_dir] + system_command! "svn", args: ["export", "--force", svn_escape(path), unpack_dir] end end end From ad18c5c4b1f1a858ed3ddae33cb7174b9f7ea78d Mon Sep 17 00:00:00 2001 From: feu Date: Fri, 27 Jul 2018 10:23:36 -0300 Subject: [PATCH 06/10] adjusting args in fetch_repo --- Library/Homebrew/download_strategy.rb | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index a7e87cb4ed..a9f221dd82 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -579,13 +579,10 @@ class SubversionDownloadStrategy < VCSDownloadStrategy # Use "svn update" when the repository already exists locally. # This saves on bandwidth and will have a similar effect to verifying the # cache as it will make any changes to get the right revision. - svncommand = target.directory? ? "update" : "checkout" - args = ["svn", svncommand] - if svncommand == "checkout" - args << url - args << target - elsif svncommand == "update" - args << svn_escape(target) + args = if target.directory? + ["svn", "update", svn_escape(target)] + else + ["svn", "checkout", url, target] end if revision ohai "Checking out #{@ref}" From ce3ac54e2c3df7b1e2e7b824f52e0c60c4298f7a Mon Sep 17 00:00:00 2001 From: feu Date: Sun, 29 Jul 2018 11:36:43 -0300 Subject: [PATCH 07/10] changing to use system_command .. chdir --- Library/Homebrew/download_strategy.rb | 9 +++++---- .../Homebrew/unpack_strategy/subversion.rb | 19 +++---------------- 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index a9f221dd82..0746387922 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -554,18 +554,19 @@ class SubversionDownloadStrategy < VCSDownloadStrategy end def source_modified_time - xml = REXML::Document.new(Utils.popen_read("svn", "info", "--xml", svn_escape(cached_location))) + info = system_command("svn", args:["info", "--xml"], chdir: cached_location.to_s).stdout + xml = REXML::Document.new(info) Time.parse REXML::XPath.first(xml, "//date/text()").to_s end def last_commit - Utils.popen_read("svn", "info", "--show-item", "revision", svn_escape(cached_location)).strip + system_command("svn", args:["info", "--show-item", "revision"], chdir: cached_location.to_s).stdout.strip end private def repo_url - Utils.popen_read("svn", "info", svn_escape(cached_location)).strip[/^URL: (.+)$/, 1] + system_command("svn", args:["info"], chdir: cached_location.to_s).stdout.strip[/^URL: (.+)$/, 1] end def externals @@ -580,7 +581,7 @@ class SubversionDownloadStrategy < VCSDownloadStrategy # This saves on bandwidth and will have a similar effect to verifying the # cache as it will make any changes to get the right revision. args = if target.directory? - ["svn", "update", svn_escape(target)] + ["cd", target.to_s, ";", "svn", "update"] else ["svn", "checkout", url, target] end diff --git a/Library/Homebrew/unpack_strategy/subversion.rb b/Library/Homebrew/unpack_strategy/subversion.rb index b185d2ae57..f3e69b0c24 100644 --- a/Library/Homebrew/unpack_strategy/subversion.rb +++ b/Library/Homebrew/unpack_strategy/subversion.rb @@ -1,20 +1,5 @@ require_relative "directory" -def svn_escape(svn_path) - # subversion uses '@' to point to a specific revision - # so when the path contains a @, it requires an additional @ at the end - # but this is not consistent through all commands - # the commands are affected as follows: - # svn checkout url1 foo@a # properly checks out url1 to foo@a - # svn switch url2 foo@a # properly switchs foo@a to url2 - # svn update foo@a@ # properly updates foo@a - # svn info foo@a@ # properly obtains info on foo@a - # svn export foo@a@ newdir # properly export foo@a contents to newdir - result = svn_path.to_s.dup - result << "@" if result.include? "@" - result -end - module UnpackStrategy class Subversion < Directory def self.can_extract?(path:, magic_number:) @@ -24,7 +9,9 @@ module UnpackStrategy private def extract_to_dir(unpack_dir, basename:, verbose:) - system_command! "svn", args: ["export", "--force", svn_escape(path), unpack_dir] + system_command! "svn", + args: ["export", "--force", ".", unpack_dir], + chdir: path.to_s end end end From 43d53a67b772a4a08748dc58e4b23f51b78fbd59 Mon Sep 17 00:00:00 2001 From: feu Date: Sun, 29 Jul 2018 20:54:13 -0300 Subject: [PATCH 08/10] just adjusting spaces --- Library/Homebrew/download_strategy.rb | 6 +++--- Library/Homebrew/unpack_strategy/subversion.rb | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index 0746387922..e0c21e273b 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -554,19 +554,19 @@ class SubversionDownloadStrategy < VCSDownloadStrategy end def source_modified_time - info = system_command("svn", args:["info", "--xml"], chdir: cached_location.to_s).stdout + info = system_command("svn", args: ["info", "--xml"], chdir: cached_location.to_s).stdout xml = REXML::Document.new(info) Time.parse REXML::XPath.first(xml, "//date/text()").to_s end def last_commit - system_command("svn", args:["info", "--show-item", "revision"], chdir: cached_location.to_s).stdout.strip + system_command("svn", args: ["info", "--show-item", "revision"], chdir: cached_location.to_s).stdout.strip end private def repo_url - system_command("svn", args:["info"], chdir: cached_location.to_s).stdout.strip[/^URL: (.+)$/, 1] + system_command("svn", args: ["info"], chdir: cached_location.to_s).stdout.strip[/^URL: (.+)$/, 1] end def externals diff --git a/Library/Homebrew/unpack_strategy/subversion.rb b/Library/Homebrew/unpack_strategy/subversion.rb index f3e69b0c24..064baf921a 100644 --- a/Library/Homebrew/unpack_strategy/subversion.rb +++ b/Library/Homebrew/unpack_strategy/subversion.rb @@ -9,7 +9,7 @@ module UnpackStrategy private def extract_to_dir(unpack_dir, basename:, verbose:) - system_command! "svn", + system_command! "svn", args: ["export", "--force", ".", unpack_dir], chdir: path.to_s end From c63d8813afe0be383d3aa98adb11112869a71122 Mon Sep 17 00:00:00 2001 From: feu Date: Mon, 30 Jul 2018 10:04:54 -0300 Subject: [PATCH 09/10] svn update/checkout with system_command --- Library/Homebrew/download_strategy.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index e0c21e273b..61774fdcee 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -580,17 +580,17 @@ class SubversionDownloadStrategy < VCSDownloadStrategy # Use "svn update" when the repository already exists locally. # This saves on bandwidth and will have a similar effect to verifying the # cache as it will make any changes to get the right revision. - args = if target.directory? - ["cd", target.to_s, ";", "svn", "update"] - else - ["svn", "checkout", url, target] - end + args = target.directory? ? ["update"] : ["checkout", url, target] if revision ohai "Checking out #{@ref}" args << "-r" << revision end args << "--ignore-externals" if ignore_externals - safe_system(*args) + if args[0] == "update" + system_command("svn", args: args, chdir: target.to_s) + else + system_command("svn", args: args) + end end def cache_tag From 4bb82ccd947f7f078e25b9e27e996fa33be73531 Mon Sep 17 00:00:00 2001 From: feu Date: Mon, 30 Jul 2018 11:12:48 -0300 Subject: [PATCH 10/10] just one target.directory check in fetch_repo --- Library/Homebrew/download_strategy.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index 61774fdcee..928e1172d9 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -580,16 +580,16 @@ class SubversionDownloadStrategy < VCSDownloadStrategy # Use "svn update" when the repository already exists locally. # This saves on bandwidth and will have a similar effect to verifying the # cache as it will make any changes to get the right revision. - args = target.directory? ? ["update"] : ["checkout", url, target] + args = [] if revision ohai "Checking out #{@ref}" args << "-r" << revision end args << "--ignore-externals" if ignore_externals - if args[0] == "update" - system_command("svn", args: args, chdir: target.to_s) + if target.directory? + system_command("svn", args: ["update", *args], chdir: target.to_s) else - system_command("svn", args: args) + system_command("svn", args: ["checkout", url, target, *args]) end end