644334 - Add hook to Bugzilla::Install::Filesystem to allow extensions to create files/directories/htaccess

Status: RESOLVED FIXED

(Bugzilla :: Extensions, defect)

Description

[David Lawrence [:dkl]]

Attachment: Patch to add install_filesystem hook for extensions (v1)

Attaching a patch adding new install_filesystem hook to allow extensions to update the data in Bugzilla::Install::Filesystem::FILESYSTEM to create files, 
directories, and htaccess files needed by the extension itself. It also allows
the permissions to be set for the new additions.

Dave

Comment 1

[Max Kanat-Alexander]

Comment on attachment 521304 [details] [diff] [review]
Patch to add install_filesystem hook for extensions (v1)

  Awesome!

  A few tiny changes:

>=== modified file 'Bugzilla/Hook.pm'
>+You will be able to also set the permissions for the files and
>+directories using this hook. 

  You are also able to set or change permissions for files and directories using this hook.

> You are also able to create appropriate
>+htaccess files for the new directory to secure it's contents.

  Should be "its" not "it's". Also replace "new" with "any".

>+For examples see L<Bugzilla::Install::Filesystem::FILESYSTEM>.

  That should be a /FILESYSTEM.

>+=item C<files>
>+
>+Hash reference of files that are already present when your extension was
>+installed but need to have specific permissions set.

  What are the keys and values?

>+Hash reference containing the name of each directory that will be created
>+pointing at its default permissions.

  Comma after "created".

>+=item C<non_recurse_dirs>
>+
>+Hash reference containing directories that we want to set the perms on, but not
>+recurse through. These are directories not created in checkesetup.pl.

  Keys and values needed.

>+=item C<recurse_dirs>
>+
>+Hash reference of directories that will have permissions set for each item inside 
>+each of these directories, including the directory itself. 

  Keys and values here, too.

>+=item C<create_files>
>+
>+Hash reference of file names to actually create pointing at it's default
>+permissions.

  Should be "its" (it's == it is, its == possessive pronoun)

  Also, that's not actually what's in create_files.

>+    # Create a new directory in datadir specifically for this extension.
>+    # The directory will need to allow files to be created by the extension
>+    # code as well as allow the webserver to server content from it.
>+    # my $data_path = bz_locations->{'datadir'} . "/" . __PACKAGE__->NAME;
>+    # $create_dirs->{$data_path} = Bugzilla::Install::Filesystem::DIR_CGI_WRITE
>+    #                              | Bugzilla::Install::Filesystem::DIR_ALSO_WS_SERVE;

  Don't encourage DIR_ALSO_WS_SERVE on data directories (remove it from the example).

>+    # Update the permissions of any files and directories that currently reside
>+    # in the extension's directory. 
>+    # $recurse_dirs->{$data_path} = {
>+    #     files => Bugzilla::Install::Filesystem::WS_SERVE,
>+    #     dirs  => Bugzilla::Install::Filesystem::DIR_CGI_WRITE
>+    #              | Bugzilla::Install::Filesystem::DIR_ALSO_WS_SERVE
>+    # };

  And similarly, let's not encourage servable data directories.

>+    # Create a htaccess file that allows specific content to be served from the 
>+    # extension's directory.
>+    # $htaccess->{"$data_path/.htaccess"} = {
>+    #     perms    => Bugzilla::Install::Filesystem::WS_SERVE,
>+    #     contents => <<EOT
>+# # Allow access to files created by the Example extension 
>+# <FilesMatch \\.html\$>
>+#   Allow from all
>+# </FilesMatch>
>+# Deny from all
>+# EOT

  Just as a normal example, let's do the same thing we do for most directories or just not add an htaccess here.

Comment 2

[David Lawrence [:dkl]]

Attachment: Patch to add install_filesystem hook for extensions (v2)

Thanks Max. Here is a new patch to hopefully address your change requests.

Dave

Comment 3

[Max Kanat-Alexander]

Comment on attachment 521657 [details] [diff] [review]
Patch to add install_filesystem hook for extensions (v2)

>=== modified file 'Bugzilla/Hook.pm'
>+You will be able to also set the permissions for the files and
>+directories using this hook.

  Remove "the" from this sentence...  

>  You are also able to set or change 
>+permissions for current files and directories using this hook.

  And then you can eliminate that sentence.

>+You are also able to create appropriate htaccess files for the 
>+any directories to secure its contents.

  You can also use this hook to create appropriate .htaccess files for any directory to secure its contents.


  Everything else looks great! The above can all be fixed on checkin.

Comment 4

[David Lawrence [:dkl]]

Thanks. Changes committed.

trunk:
Committing to: bzr+ssh://[e-mail]/bugzilla/trunk
modified Bugzilla/Hook.pm
modified Bugzilla/Install/Filesystem.pm
modified extensions/Example/Extension.pm
Committed revision 7764.

4.0:
Committing to: bzr+ssh://[e-mail]/bugzilla/4.0
modified Bugzilla/Hook.pm
modified Bugzilla/Install/Filesystem.pm
modified extensions/Example/Extension.pm
Committed revision 7567.