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

Support spaces in git repository path #1066

Merged
merged 4 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 16 additions & 14 deletions lib/kamal/commands/builder/clone.rb
Original file line number Diff line number Diff line change
@@ -1,29 +1,31 @@
module Kamal::Commands::Builder::Clone
extend ActiveSupport::Concern

included do
delegate :clone_directory, :build_directory, to: :"config.builder"
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This delegation becomes superfluous when we need to escape the values returned by clone_directory and build_directory, so I just removed it. Inlined config.builder.clone_directory.shellescape in the single location it's needed, and factorized config.builder.build_directory.shellescape in the escaped_build_directory method.


def clone
git :clone, Kamal::Git.root, "--recurse-submodules", path: clone_directory
git :clone, escaped_root, "--recurse-submodules", path: config.builder.clone_directory.shellescape
end

def clone_reset_steps
[
git(:remote, "set-url", :origin, Kamal::Git.root, path: build_directory),
git(:fetch, :origin, path: build_directory),
git(:reset, "--hard", Kamal::Git.revision, path: build_directory),
git(:clean, "-fdx", path: build_directory),
git(:submodule, :update, "--init", path: build_directory)
git(:remote, "set-url", :origin, escaped_root, path: escaped_build_directory),
git(:fetch, :origin, path: escaped_build_directory),
git(:reset, "--hard", Kamal::Git.revision, path: escaped_build_directory),
git(:clean, "-fdx", path: escaped_build_directory),
git(:submodule, :update, "--init", path: escaped_build_directory)
]
end

def clone_status
git :status, "--porcelain", path: build_directory
git :status, "--porcelain", path: escaped_build_directory
end

def clone_revision
git :"rev-parse", :HEAD, path: build_directory
git :"rev-parse", :HEAD, path: escaped_build_directory
end

def escaped_root
Kamal::Git.root.shellescape
end

def escaped_build_directory
config.builder.build_directory.shellescape
end
end
19 changes: 15 additions & 4 deletions test/commands/builder_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -149,15 +149,26 @@ class CommandsBuilderTest < ActiveSupport::TestCase
assert_equal "docker info --format '{{index .RegistryConfig.Mirrors 0}}'", command.first_mirror.join(" ")
end

test "clone path with spaces" do
command = new_builder_command
Kamal::Git.stubs(:root).returns("/absolute/path with spaces")
clone_command = command.clone.join(" ")
clone_reset_commands = command.clone_reset_steps.map { |a| a.join(" ") }

assert_match(%r{path\\ with\\ space}, clone_command)
assert_no_match(%r{path with spaces}, clone_command)

clone_reset_commands.each do |command|
assert_match(%r{path\\ with\\ space}, command)
assert_no_match(%r{path with spaces}, command)
end
end

private
def new_builder_command(additional_config = {})
Kamal::Commands::Builder.new(Kamal::Configuration.new(@config.deep_merge(additional_config), version: "123"))
end

def build_directory
"#{Dir.tmpdir}/kamal-clones/app/kamal/"
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wondered what this was, and realized it has become unnecessary when tests still passed without it.

def local_arch
Kamal::Utils.docker_arch
end
Expand Down