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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CUSTDB-465] phpBB Package Builder #299

Draft
wants to merge 1 commit into
base: 3.2.x
Choose a base branch
from

Conversation

battye
Copy link
Member

@battye battye commented Oct 13, 2019

Overview

This allows users to download a pre-packaged version of phpBB3 which contains extensions, language packs and styles which they have already selected within Titania.

This package builder code is still a work in progress. I wanted to open it up for discussion because I have a few questions which I need clarification on before proceeding further.

The general idea is that a user can go through the customisations (extensions, language packs and styles approved for v3.2) in Titania, and when they see something the like instead of downloading it standalone they can "add" it to a package.

When they add items, it gets stored in a cookie (with an expiry of an hour). At any time after adding something, the user will be given the option of downloading the full package.

The full package will be a zip of the latest version of phpBB3, with all of the customisations the user selected automatically put into place within the folder structure.

image

My Questions

Language packs

@Crizz0 Would you be able to supply the convention of where directories from a language pack need to go? For example:

  • If we unzip Eesti-keelne-phpBB-3.2.8.zip that creates a Eesti-keelne-phpBB-3.2.8 directory, and within that is a Eesti keelne phpBB 3.2.8 directory. Is that the standard format translators use, or do some put all of their files at the top level?
  • \ext\phpbb\viglink\language\{iso} => \ext\phpbb\viglink\language\{iso}
  • \language\{iso} => \language\{iso}
  • \styles\prosilver\theme\{iso} => \styles\prosilver\theme\{iso}
    • Do translators ever put theme icons for anything besides prosilver? Because we'd need to account for that if a user was to pre-package a style.

Styles

@phpbb-es @vinny Are there any times when style authors do something unpredictable? Any special rules I should know about when automatically copying a style into the appropriate folder within phpBB?

  • For example if I download Blackfog_3.2.8.2.zip and unzip it, a Blackfog_3.2.8.2 will be created which contains a subfolder called Blackfog. Which is then copied to the style/ directory within phpBB. Can I rely on it being that simple for every style?

Extensions

@kasimi @VSEphpbb @DavidIQ Can you think of anything users might do inside their own extension directories which would potentially be a problem when writing rules to copy extension code out of an extension zip into a phpBB zip?

General

Can anyone anticipate any problems by having extensions, language packs and styles already put into place within the directory structure before the user has installed phpBB?

Potential problems

After unzipping the latest phpBB version (which we have inside Titania) into /phpBB3/ext/phpbb/titania/files/contrib_temp/tmp I noticed it creates the following error - I believe it might be a composer/autoloader issue because it's finding multiple copies of the phpBB codebase? If anyone can think of a workaround for this, some way that this directory can be ignored for composer purposes, please let me know. Ultimately we need some place on the file system where phpBB3 can be unzipped, extensions/language packs/style can be unzipped and then all of the files moved into the proper location before phpBB3 is zipped back up again, and all of the files are cleaned up.

Whoops, looks like something went wrong.
FatalErrorException in collection_pass.php line 24:
Compile Error: Cannot declare class phpbb\di\pass\collection_pass, because the name is already in use
in collection_pass.php line 24
in class_loader.php line 160
at class_loader->load_class() in container_builder.php line 660
at spl_autoload_call() in container_builder.php line 660
at class_exists() in container_builder.php line 660
at container_builder->register_ext_compiler_pass() in container_builder.php line 180
at container_builder->get_container() in common.php line 115
in app.php line 23
at {main}() in app.php line 0

To be completed

UI

At the moment, the only change to the UI is what I've shown above. There is a [+] button next to a revision to add that to the package, and after the AJAX call has updated the cookie that turns into a "Download package with {x} customisations" link. If the user leaves and returns to that customisation page, the plus sign will no longer be visible as a revision has already been flagged for packaging.

It might be nice if we had a new page which a user could visit that shows their selections, and gave them the ability to remove things they've added (or clear all options, which I've already added the /package-builder/reset route for.

If anyone has any suggestions please post them here. Just remember I have zero skills with images so anything that involves new icons might be a non-starter unless someone can make them for me 馃槢

Downloading the final zip

This is still a work in progress due to the problems mentioned above. I need to have a hard set of rules/conventions such that whenever Titania unzips an extension, language pack or style it knows what to copy and to where.

I also need to add some authorisation logic around the revision/contrib ID's before downloading so that someone can't manipulate the cookie and download a revision they don't have access to (like something unapproved).

Status

  • Allow users to select customisations for pre-packing
  • Save selections to a cookie
  • Show download link for final package
  • Finalise rules for packaging extensions
  • Finalise rules for packaging language packs
  • Finalise rules for packaging styles
  • Combine all of the customisations into the package and download to the browser as a zip
  • Create a new page where people can view/remove their selections and another download link

https://tracker.phpbb.com/projects/CUSTDB/issues/CUSTDB-465

@vinny
Copy link
Member

vinny commented Oct 13, 2019

For example if I download Blackfog_3.2.8.2.zip and unzip it, a Blackfog_3.2.8.2 will be created which contains a subfolder called Blackfog. Which is then copied to the style/ directory within phpBB. Can I rely on it being that simple for every style?

All styles in the CBD will be unzip in their respective directories. Example:

  • prosilver_3.2.8.zip unzip to prosilver
  • Blackfog_3.2.8.2.zip unzip to Blackfog
  • black_2.0.0.zip unzip to black

@paul999
Copy link
Member

paul999 commented Oct 13, 2019

See for packaging for extensions the actual rules: https://www.phpbb.com/extensions/rules-and-policies/validation-policy/#packaging
So there can't be anything outside the ext/vendor/name folder when downloading an extension. Every extension will include the vendor/name directory in the zip, from the CDB. These rules have been in place since we started doing extensions, and we really should not change them, as that will be a huge headache.

Also, I wouldn't use cookies for storing stuff, but instead add a new columns to sessions table and use that.

Another thing that might need discussion is regarding support. I know the support team always says we only provide support for official packages from phpbb.com, but this sort of adds a new dimention to that.

@paul999
Copy link
Member

paul999 commented Oct 13, 2019

After unzipping the latest phpBB version (which we have inside Titania) into /phpBB3/ext/phpbb/titania/files/contrib_temp/tmp I noticed it creates the following error

You violate the extensions validation policy with that :).
But yeah, any file in ext/ will be included in autoload I think. It is always suggested to use store/ for these kind of stuff, as that directory is also writeable by default (And which ext/ isn't :)).

@battye
Copy link
Member Author

battye commented Oct 13, 2019

Thanks for your replies.

@vinny - is there a convention around the naming? eg. Blackfog_3.2.8.2.zip unzip to Blackfog will the name from the beginning of the zip filename always be the name of the directory? Will it always be the same as the contrib name?

@paul999 - what do you think are the pros and cons of cookies vs the session table?
Regarding contrib_temp wasn't that originally being used as a storage folder for repacking anyway?

@paul999
Copy link
Member

paul999 commented Oct 13, 2019

The reason why I would prefer sessions over cookies is that phpBB has a whole handling of sessions already build in, and you can just get it directly from $user->data without doing anything specific. But I guess this is just a personal preference.

I dunno regarding the contrib_temp directory specificly, but in this specific case I can see causing it issues. I know this extension doesn't need to meet the CDB rules, but it would be a good thing if it does meet it. Plus you wouln't have the autoload issues as store/ isn't included in autoload

@battye
Copy link
Member Author

battye commented Oct 13, 2019

I think you're right about the contrib_temp directory, it's definitely not going to work seeing it's autoloaded. I'll need to have a look at my CLI scripts which used it. There's a number of places it's used besides that too that I'm not familiar with.

Given that, at least in this case, all it's being used for is a place to dump some files in order to move things around, would it be better to use somewhere on the filesystem like /tmp instead of phpBB's /store directory?

@paul999
Copy link
Member

paul999 commented Oct 13, 2019

You don't know which directory is /tmp. While it might work on all linux based machines, it won't work on windows :). Having it just in store/ will work on any OS, and as it is a required for phpBB to have it writeable, you won't need users to configure anything specific.
In the policies we suggest to use store/name/vendor, https://www.phpbb.com/extensions/rules-and-policies/validation-policy/#other-uploaded-files , so I would just use that, and create a tmp/ folder in there.

@battye
Copy link
Member Author

battye commented Oct 13, 2019

That's a good approach, let's do that 馃憤

@vinny
Copy link
Member

vinny commented Oct 13, 2019

@vinny - is there a convention around the naming? eg. Blackfog_3.2.8.2.zip unzip to Blackfog will the name from the beginning of the zip filename always be the name of the directory? Will it always be the same as the contrib name?

https://www.phpbb.com/styles/rules-and-policies/submission-policy/3.2/
We have the rules about naming the directory and the zip archive. Even with the recommendations, some authors change the name of the zip, which sometimes differs from the style directory.
But anyway, the zip file extracts the style directory, for example:

  • style_name_3.2.8.zip
    -- style_name
    --- template
    --- theme
    --- style.cfg

@phpbb-es
Copy link
Member

@battye the /contrib/ directory it's always optional...

The structure for your board style submission should be as follows (using example "prosilver"):
/prosilver/
contrib/ (optional)
template/
theme/
style.cfg

@Crizz0
Copy link
Member

Crizz0 commented Oct 13, 2019

If we unzip Eesti-keelne-phpBB-3.2.8.zip that creates a Eesti-keelne-phpBB-3.2.8 directory, and within that is a Eesti keelne phpBB 3.2.8 directory. Is that the standard format translators use, or do some put all of their files at the top level?
\ext\phpbb\viglink\language\{iso} => \ext\phpbb\viglink\language\{iso}
\language\{iso} => \language\{iso}
\styles\prosilver\theme\{iso} => \styles\prosilver\theme\{iso}
    Do translators ever put theme icons for anything besides prosilver? Because we'd need to account for that if a user was to pre-package a style.

I try to answer that:

  • The translation packages look always like that: german_casual_3_2_8.zip > german_casual_3_2_8/ > languages/, styles/, ext/
  • This contains the files, which needed to be translated. I can include some CSS, usually only one image (the online image) and that's it.
  • No, I did not see yet any other files for other styles, since subsilver2 was dropped.

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

Successfully merging this pull request may close these issues.

None yet

5 participants