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

Add ability to exclude certain files from the Packager #37

Closed
quicksketch opened this issue May 31, 2019 · 8 comments · Fixed by #38
Closed

Add ability to exclude certain files from the Packager #37

quicksketch opened this issue May 31, 2019 · 8 comments · Fixed by #38

Comments

@quicksketch
Copy link
Member

When packaging zip files, we currently get a zip from GitHub, unzip, modify all .info files, and then rezip them all into a new archive.

During this process, it would be nice to be able to remove certain blacklisted files, such as CI scripts like .travis.yml, .zenci.yml, etc.

The packager currently lives in project_github.module, in _project_github_create_package().

@docwilmot
Copy link
Member

PR adds an alter hook so other modules can do this.
Coincidentally needed for removing the screenshots folder from themes, mentioned also here backdrop-ops/backdropcms.org#487 (comment)

@docwilmot
Copy link
Member

@quicksketch please let me know if a simple hook like this will do the trick. This blocks the work I linked to above so would appreciate a look. I'm also wondering if the alter should also pass the directory url to make it easier to build full urls in implementations.

@quicksketch
Copy link
Member Author

The hook looks great! There's a small issue in the example documentation though, could you replace the $errors array or just remove it entirely?

@quicksketch
Copy link
Member Author

I'm also wondering if the alter should also pass the directory url to make it easier to build full urls in implementations.

Not sure about this. The current hook seems like it would do the job. Could you provide a further example that would use the directory path?

@docwilmot
Copy link
Member

On Gitter:

@quicksketch to recap, this hook would help to push through adding more data in info files. backdrop-ops/backdropcms.org#487 (comment)
My PR for this would want to rmove screenshot directories.
The code to do that in another PR I have waiting is like this:
[In an implementation of this new hook:]

  foreach ($files as $path => $file) {
    // Find the base .info file. If the file is a .info file, and it is named
    // the same as the project, then we use this file, even if there are 
    // multiple .info files in the package.
    $extension = substr($file->filename, strrpos($file->filename, '.') + 1);
    if ($extension === 'info' && $file->name == $project_name) {
      $info_contents = backdrop_parse_info_file($file->uri);
      $directory_path = rtrim($path, $file->filename);
      if (is_dir($directory_path . 'screenshots')) {
        $screenshots_directory_path = $directory_path . 'screenshots/';
      }
    }
  }

See $directory_path = rtrim($path, $file->filename);. To get the file path for everything, I'm deducing it by subtracting the file name from the array keys of the $files array.
Not sure if that will always be reliable, and I think using this new hook to do any file operations would maybe be more reliable if we passed the directory path where those $files are

@quicksketch reply:
@docwilmot Hm, I think because you can already derive the directory path from the $files array we should not pass it as another parameter.

@quicksketch
Copy link
Member Author

Looks great! I merged #38 into 1.x-1.x branch. Thanks!

@docwilmot
Copy link
Member

Woohoo!

@quicksketch
Copy link
Member Author

That said, we have a parallel discussion at #33 that might change this. I'll cross link over here and see if there are use-cases for which we need to adjust.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants