diff --git a/Changes b/Changes index f308db4..f70ab2d 100644 --- a/Changes +++ b/Changes @@ -1,3 +1,12 @@ +v1.20.1.beta.1 - 2014/10/x + + Bug fixes: + + - During publishing, now checks asset names after file name globbing, not + before. Also, archive names are checked for extraction support prior to + writing to remote repository. (Yes, that's the archive /names/ and not the + archive /files/.) + v1.20.0 - 2014/10/7 New features: diff --git a/TODO.md b/TODO.md index 9498d42..4300798 100644 --- a/TODO.md +++ b/TODO.md @@ -1,6 +1,7 @@ # Code changes * Retrieve statements should validate their paths the same way that path statements do. +* Require Ruby v1.9.3 and fix those bits that work around earlier versions. (Run "`ack -Q 1.9.3`".) * Produce nice error messages when packages contain file names that Windows can't deal with. * Periodically `ack '\bTODO:'` and fix what we can. diff --git a/lib/fig/repository_package_publisher.rb b/lib/fig/repository_package_publisher.rb index 7523989..cbc1746 100644 --- a/lib/fig/repository_package_publisher.rb +++ b/lib/fig/repository_package_publisher.rb @@ -56,14 +56,16 @@ def package_statements=(statements) def publish_package() derive_publish_metadata() - validate_asset_names() temp_dir = publish_temp_dir() @operating_system.delete_and_recreate_directory(temp_dir) - @operating_system.delete_and_recreate_directory(@local_directory_for_package) + @operating_system.delete_and_recreate_directory( + @local_directory_for_package + ) fig_file = File.join(temp_dir, Fig::Repository::PACKAGE_FILE_IN_REPO) - content, published_package = derive_definition_file + content, published_package = + derive_definition_file_and_create_resource_archive @operating_system.write(fig_file, content) publish_package_contents @@ -101,34 +103,11 @@ def derive_login() return Etc.getlogin() end - def validate_asset_names() - asset_statements = @text_assembler.asset_input_statements - - asset_names = Set.new() - asset_statements.each do - |statement| - - asset_name = statement.asset_name() - if not asset_name.nil? - if asset_name == Fig::Repository::RESOURCES_FILE - Fig::Logging.fatal \ - %Q - end - - if asset_names.include?(asset_name) - Fig::Logging.fatal \ - %Q - raise Fig::RepositoryError.new - else - asset_names.add(asset_name) - end - end - end - end - - def derive_definition_file() + def derive_definition_file_and_create_resource_archive() add_package_metadata_comments() - add_output_statements_and_create_resource_archive() + assemble_output_statements_and_accumulate_assets() + validate_asset_names() + create_resource_archive() add_unparsed_text() file_content, explanations = @text_assembler.assemble_package_definition() @@ -136,7 +115,7 @@ def derive_definition_file() explanations.each {|explanation| Fig::Logging.info explanation} end - published_package = nil + to_be_published_package = nil begin unparsed_package = Fig::NotYetParsedPackage.new unparsed_package.descriptor = @descriptor @@ -146,7 +125,7 @@ def derive_definition_file() unparsed_package.source_description = '' unparsed_package.unparsed_text = file_content - published_package = + to_be_published_package = Fig::Parser.new(nil, false).parse_package(unparsed_package) rescue Fig::PackageParseError => error raise \ @@ -154,7 +133,7 @@ def derive_definition_file() "#{error}\n\nGenerated contents:\n#{file_content}" end - return file_content, published_package + return file_content, to_be_published_package end def add_package_metadata_comments() @@ -353,19 +332,9 @@ def get_git_sha1() return reference end - # Deals with Archive and Resource statements. It downloads any remote - # files (those where the statement references a URL as opposed to a local - # file) and then copies all files into the local repository and the remote - # repository (if not a local-only publish). - def add_output_statements_and_create_resource_archive() - assemble_output_statements() - create_resource_archive() - - return - end - - def assemble_output_statements() - @resource_paths = [] + def assemble_output_statements_and_accumulate_assets() + @local_resource_paths = [] + @asset_names_with_associated_input_statements = {} @text_assembler.input_statements.each do |statement| @@ -382,39 +351,121 @@ def assemble_output_statements() def add_asset_to_output_statements(asset_statement) if Fig::URL.is_url? asset_statement.location - @text_assembler.add_output asset_statement + add_output_asset_statement_based_upon_input_statement( + asset_statement, asset_statement + ) elsif asset_statement.is_a? Fig::Statement::Archive if asset_statement.requires_globbing? expand_globs_from( [asset_statement.location] ).each do |file| - @text_assembler.add_output( + add_output_asset_statement_based_upon_input_statement( Fig::Statement::Archive.new( nil, %Q<[synthetic statement created in #{__FILE__} line #{__LINE__}]>, file, false # No globbing - ) + ), + asset_statement ) end else - @text_assembler.add_output asset_statement + add_output_asset_statement_based_upon_input_statement( + asset_statement, asset_statement + ) end elsif asset_statement.requires_globbing? - @resource_paths.concat expand_globs_from( [asset_statement.location] ) + @local_resource_paths.concat expand_globs_from( + [asset_statement.location] + ) else - @resource_paths << asset_statement.location + @local_resource_paths << asset_statement.location end return end + def add_output_asset_statement_based_upon_input_statement( + output_statement, input_statement + ) + @text_assembler.add_output output_statement + + asset_name = output_statement.asset_name + + input_statements = + @asset_names_with_associated_input_statements[asset_name] + if input_statements.nil? + input_statements = [] + + @asset_names_with_associated_input_statements[asset_name] = + input_statements + end + + input_statements << input_statement + + return + end + + def validate_asset_names() + found_problem = false + + @asset_names_with_associated_input_statements.keys.sort.each do + |asset_name| + + statements = @asset_names_with_associated_input_statements[asset_name] + + if asset_name == Fig::Repository::RESOURCES_FILE + Fig::Logging.fatal \ + %Q + found_problem = true + end + + archive_statements = + statements.select { |s| s.is_a? Fig::Statement::Archive } + if ( + not archive_statements.empty? \ + and not ( + asset_name =~ /\.tar\.gz$/ \ + or asset_name =~ /\.tgz$/ \ + or asset_name =~ /\.tar\.bz2$/ \ + or asset_name =~ /\.zip$/ + ) + ) + Fig::Logging.fatal \ + %Q + found_problem = true + end + + if statements.size > 1 + Fig::Logging.fatal \ + %Q + found_problem = true + end + end + + if found_problem + raise Fig::RepositoryError.new + end + end + + def collective_position_string(statements) + return ( + statements.map { + |statement| + + position_string = statement.position_string + + position_string.empty? ? '' : position_string.strip + } + ).join(', ') + end + def create_resource_archive() - if @resource_paths.size > 0 - check_asset_paths(@resource_paths) + if @local_resource_paths.size > 0 + check_asset_paths(@local_resource_paths) file = File.join publish_temp_dir, Fig::Repository::RESOURCES_FILE - @operating_system.create_archive(file, @resource_paths) + @operating_system.create_archive(file, @local_resource_paths) Fig::AtExit.add { FileUtils.rm_f(file) } @text_assembler.add_output( diff --git a/spec/command/grammar_asset_spec.rb b/spec/command/grammar_asset_spec.rb index 2b03b2f..ee3710c 100644 --- a/spec/command/grammar_asset_spec.rb +++ b/spec/command/grammar_asset_spec.rb @@ -11,7 +11,7 @@ def test_published_asset_with_url_with_symbol( asset_type, symbol, quote, version ) - file_name = "with#{symbol}symbol" + file_name = "with#{symbol}symbol.zip" escaped_file = CGI.escape file_name quoted_url = "#{quote}file://#{CURRENT_DIRECTORY}/#{escaped_file}#{quote}" @@ -85,7 +85,7 @@ def test_published_file_asset_with_url_with_symbol( def test_published_asset_with_file_with_symbol( asset_type, symbol, quote, version ) - file_name = "with#{symbol}symbol" + file_name = "with#{symbol}symbol.zip" quoted_name = "#{quote}#{file_name}#{quote}" it %Q do @@ -176,10 +176,10 @@ def testable_v0_special_characters() |quote| begin - value = "#{quote}nothing-special#{quote}" + value = "#{quote}nothing-special.zip#{quote}" it %Q do - write_file "#{CURRENT_DIRECTORY}/nothing-special", '' + write_file "#{CURRENT_DIRECTORY}/nothing-special.zip", '' fig( [%w< --publish foo/1.2.3 --set x=y >, "--#{asset_type}", value], @@ -259,10 +259,10 @@ def testable_v0_special_characters() |quote| begin - value = "#{quote}nothing-special#{quote}" + value = "#{quote}nothing-special.zip#{quote}" it %Q do - write_file "#{CURRENT_DIRECTORY}/nothing-special", '' + write_file "#{CURRENT_DIRECTORY}/nothing-special.zip", '' input = <<-"END" grammar v1 diff --git a/spec/command/publishing_spec.rb b/spec/command/publishing_spec.rb index 822f497..9c6fee1 100644 --- a/spec/command/publishing_spec.rb +++ b/spec/command/publishing_spec.rb @@ -25,17 +25,17 @@ out, err, exit_code = fig(%w<--publish foo/1.2.3>, input, :no_raise_on_error => true) - err.should =~ /multiple archives/ + err.should =~ /multiple assets/ err.should =~ /duplicate-archive\.tar\.gz/ exit_code.should_not == 0 end - it 'complains when archives are named the same as the resources tarball' do - %w< archive resource >.each do - |statement_type| + %w< archive resource >.each do + |asset_type| + it "complains when #{asset_type}s refer to a URL with a base name the same as the resources tarball" do input = <<-END - #{statement_type} http://some-host/#{Fig::Repository::RESOURCES_FILE} + #{asset_type} http://some-host/#{Fig::Repository::RESOURCES_FILE} config default end END @@ -46,10 +46,6 @@ /cannot have an asset with the name "#{Regexp.escape(Fig::Repository::RESOURCES_FILE)}"/ exit_code.should_not == 0 end - end - - %w< archive resource >.each do - |asset_type| it "complains when #{asset_type}s refer to non-existent local paths" do out, err, exit_code = @@ -57,7 +53,7 @@ [ %w<--publish foo/1.2.3>, "--#{asset_type}", - 'does not exist', + 'does not exist.zip', %w<--set VARIABLE=VALUE> ].flatten, :no_raise_on_error => true @@ -69,6 +65,24 @@ end end + it "complains when globbing for archives picks up a file with the same as the resources tarball" do + write_file( + "#{CURRENT_DIRECTORY}/#{Fig::Repository::RESOURCES_FILE}", '' + ) + + input = <<-END + archive * + config default end + END + + out, err, exit_code = + fig(%w<--publish foo/1.2.3>, input, :no_raise_on_error => true) + + err.should =~ + /cannot have an asset with the name "#{Regexp.escape(Fig::Repository::RESOURCES_FILE)}"/ + exit_code.should_not == 0 + end + it 'publishes to remote repository' do input = <<-END config default diff --git a/spec/command/usage_errors_spec.rb b/spec/command/usage_errors_spec.rb index 2163c23..d22b9fa 100644 --- a/spec/command/usage_errors_spec.rb +++ b/spec/command/usage_errors_spec.rb @@ -330,6 +330,33 @@ exit_code.should_not == 0 out.should == '' end + + it %q do + archive_file = 'unknown.archive-type' + write_file "#{CURRENT_DIRECTORY}/#{archive_file}", '' + + input = <<-END + grammar v0 + + archive #{archive_file} + + config default + end + END + + out, err, exit_code = fig( + %w<--publish package/version>, + input, + :fork => false, + :no_raise_on_error => true + ) + + err.should =~ %r< \b #{ Regexp.escape(archive_file) } \b >ix + err.should =~ %r< \b unknown [ ] archive [ ] type \b >ix + + exit_code.should_not == 0 + out.should == '' + end end it %q do