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

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 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 commented May 2, 2019

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

@tonytonyjan
Copy link
Contributor Author

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
@hsbt
Copy link
Member

hsbt commented Aug 17, 2019

Seems good. Thanks!

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

Successfully merging this pull request may close these issues.

3 participants