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

Make PackageTask be able to omit parent directory while packing files #310

Merged
merged 1 commit into from Aug 17, 2019

Conversation

@tonytonyjan
Copy link
Contributor

@tonytonyjan tonytonyjan commented Apr 19, 2019

Expected

$ unzip -l pkg/my_browser_extension-1.0.1.zip
Archive:  pkg/my_browser_extension-1.0.1.zip
  Length      Date    Time    Name
---------  ---------- -----   ----
      469  04-17-2019 17:02   manifest.json
        0  04-19-2019 17:57   src/
     1417  04-19-2019 11:55   src/main.js
        0  04-19-2019 17:57   src/icons/
     1946  04-17-2019 12:28   src/icons/48.png
     5262  04-17-2019 12:29   src/icons/128.png
      625  04-17-2019 12:28   src/icons/16.png
---------                     -------
     9719                     7 files

Actual

$ unzip -l pkg/my_browser_extension-1.0.1.zip
Archive:  pkg/my_browser_extension-1.0.1.zip
  Length      Date    Time    Name
---------  ---------- -----   ----
        0  04-19-2019 17:57   my_browser_extension-1.0.1/
      469  04-17-2019 17:02   my_browser_extension-1.0.1/manifest.json
        0  04-19-2019 17:57   my_browser_extension-1.0.1/src/
     1417  04-19-2019 11:55   my_browser_extension-1.0.1/src/main.js
        0  04-19-2019 17:57   my_browser_extension-1.0.1/src/icons/
     1946  04-17-2019 12:28   my_browser_extension-1.0.1/src/icons/48.png
     5262  04-17-2019 12:29   my_browser_extension-1.0.1/src/icons/128.png
      625  04-17-2019 12:28   my_browser_extension-1.0.1/src/icons/16.png
---------                     -------
     9719                     8 files

Why Do We Need This Feature?

The problem emerged when I want to use PackageTask to package my Chrome/Firefox extension. PackageTask can only package the whole directory but Mozilla does not accept such structure:

The ZIP file must be a ZIP of the extension's files themselves, not of the directory containing them.

https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Package_your_extension_

Before (Workaround)

There is no way to omit parent directory by set config to PackageTask, alternatively below is a workaround to achieve this:

Rake::PackageTask.new('my_browser_extension', version) do |pkg|
  pkg.package_files.include('src/**/*', 'manifest.json')
  file "#{pkg.package_dir}/#{pkg.zip_file}" => [pkg.package_dir_path] + pkg.package_files do
    chdir(pkg.package_dir_path) { sh pkg.zip_command, '-r', pkg.zip_file, '.' }
    mv "#{pkg.package_dir_path}/#{pkg.zip_file}", pkg.package_dir
  end
  task default: "#{pkg.package_dir}/#{pkg.zip_file}"
end

After

With this pull request, we can now make it easier:

Rake::PackageTask.new('my_browser_extension', version) do |pkg|
  pkg.package_files.include('src/**/*', 'manifest.json')
  pkg.need_zip = true
  pkg.without_parent_dir = true
end
@coveralls
Copy link

@coveralls coveralls commented Apr 19, 2019

Coverage Status

Coverage decreased (-0.1%) to 97.438% when pulling 382e804 on tonytonyjan:without_parent_dir into 1c22b49 on ruby:master.

@hsbt
Copy link
Member

@hsbt hsbt commented May 2, 2019

@tonytonyjan Can you add a test-case with without_parent_dir option?

@tonytonyjan tonytonyjan force-pushed the tonytonyjan:without_parent_dir branch from 2b1151a to 382e804 May 10, 2019
@tonytonyjan
Copy link
Contributor Author

@tonytonyjan tonytonyjan commented May 10, 2019

Hi, @hsbt, I added a new test case with the following diff.

Since I did not see any IO or shell-like operation in the test file, I infer that I should only write tests for #working_dir and #target_dir. Actually, I am not pretty sure about how to do it correctly.

If you have any opinion about the test, please let me know.

Thanks for reviewing. :)

diff --git a/test/test_rake_package_task.rb b/test/test_rake_package_task.rb
index d3886f8..25a1baa 100644
--- a/test/test_rake_package_task.rb
+++ b/test/test_rake_package_task.rb
@@ -78,4 +78,16 @@ class TestRakePackageTask < Rake::TestCase # :nodoc:
     assert_equal "a", pkg.package_name
   end

+  def test_without_parent_dir
+    pkg = Rake::PackageTask.new("foo", :noversion)
+
+    assert_equal "pkg", pkg.working_dir
+    assert_equal "foo", pkg.target_dir
+
+    pkg.without_parent_dir = true
+
+    assert_equal "pkg/foo", pkg.working_dir
+    assert_equal ".", pkg.target_dir
+  end
+
 end
@hsbt hsbt merged commit 7eff2ab into ruby:master Aug 17, 2019
5 checks passed
5 checks passed
codeclimate All good!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.1%) to 97.438%
Details
@azure-pipelines
ruby.rake Build #20190510.1 succeeded
Details
@hsbt
Copy link
Member

@hsbt hsbt commented Aug 17, 2019

Seems good. Thanks!

@tonytonyjan tonytonyjan deleted the tonytonyjan:without_parent_dir branch Aug 18, 2019
This was referenced Mar 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants