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

Fix configuring source path globs that expand into a single directory #312

Merged
merged 2 commits into from
Sep 24, 2020
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
6 changes: 6 additions & 0 deletions lib/licensed/commands/command.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,12 @@ def default_reporter(options)
# Returns whether the command succeeded for the application.
def run_app(app)
reporter.report_app(app) do |report|
# ensure the app source path exists before evaluation
if !Dir.exist?(app.source_path)
report.errors << "No such directory #{app.source_path}"
next false
end

Dir.chdir app.source_path do
begin
# allow additional report data to be given by commands
Expand Down
25 changes: 14 additions & 11 deletions lib/licensed/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -158,29 +158,32 @@ def initialize(options = {})
def self.expand_app_source_path(app_config)
return app_config if app_config["source_path"].to_s.empty?

# check if the source path maps to an existing directory
source_path = File.expand_path(app_config["source_path"], AppConfiguration.root_for(app_config))
return app_config if Dir.exist?(source_path)

# try to expand the source path for glob patterns
expanded_source_paths = Dir.glob(source_path).select { |p| File.directory?(p) }
# return the original configuration if glob didn't result in multiple paths
return app_config if expanded_source_paths.size <= 1
configs = expanded_source_paths.map { |path| app_config.merge("source_path" => path) }

# map the expanded paths to new application configurations
expanded_source_paths.map do |path|
config = app_config.merge("source_path" => path)
# if no directories are found for the source path, return the original config
return app_config if configs.size == 0

# update configured values for name and cache_path for uniqueness.
# this is only needed when values are explicitly set, AppConfiguration
# will handle configurations that don't have these explicitly set
dir_name = File.basename(path)
# update configured values for name and cache_path for uniqueness.
# this is only needed when values are explicitly set, AppConfiguration
# will handle configurations that don't have these explicitly set
configs.each do |config|
dir_name = File.basename(config["source_path"])
config["name"] = "#{config["name"]}-#{dir_name}" if config["name"]

# if a cache_path is set and is not marked as shared, append the app name
# to the end of the cache path to make a unique cache path for the app
if config["cache_path"] && config["shared_cache"] != true
config["cache_path"] = File.join(config["cache_path"], dir_name)
end

config
end

configs
end

# Find a default configuration file in the given directory.
Expand Down
13 changes: 13 additions & 0 deletions test/commands/command_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,19 @@
end
end

it "catches and reports a non-existent app source path" do
nonexistent_path = File.join(Dir.pwd, "nonexistent")
apps.each { |app| app["source_path"] = nonexistent_path }

refute command.run

reports = command.reporter.report.all_reports.select { |r| r.target.is_a?(Licensed::AppConfiguration) }
refute_empty reports
reports.each do |r|
assert_includes r.errors, "No such directory #{nonexistent_path}"
end
end

it "allows implementations to add extra data to reports with a yielded block" do
assert command.run

Expand Down
18 changes: 18 additions & 0 deletions test/configuration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@
assert_equal 2, config.apps.size
assert_equal "app1", config.apps[0]["name"]
assert_equal "app2", config.apps[1]["name"]

config.apps.each do |app|
assert_equal File.expand_path("../../", __FILE__), app["source_path"]
end
end

it "includes default options" do
Expand Down Expand Up @@ -140,6 +144,20 @@
end
end

it "expands source_path patters that result in a single path" do
apps.clear
apps << { "source_path" => File.expand_path("../../lib/*", __FILE__) }
expected_source_paths = Dir.glob(apps[0]["source_path"]).select { |p| File.directory?(p) }
expected_source_paths.each do |source_path|
app = config.apps.find { |a| a["source_path"] == source_path }
assert app
dir_name = File.basename(source_path)
assert_equal app.root.join(Licensed::AppConfiguration::DEFAULT_CACHE_PATH, dir_name),
app.cache_path
assert_equal dir_name, app["name"]
end
end

it "assigns uniqueness to explicitly configured values" do
cache_path = ".test_licenses"
name = "test"
Expand Down