Skip to content

Conversation

@PhilETaylor
Copy link
Contributor

Pull Request for Issue #33247

Steps to reproduce the issue

Click Update Structure in Joomla 4

Screenshot 2021-04-22 at 18 52 14

Expected result

Composer provided libraries in libraries/vendor are untouched !

Actual result

FTP reveals two files get deleted.. Thats because they are listed in com_admin/script.php

/var/www/html/libraries/vendor/paragonie/sodium_compat/src/Core32/Curve25519/README.md
/var/www/html/libraries/vendor/typo3/phar-stream-wrapper/composer.json
Thu Apr 22 17:51:04 2021 [pid 75859] CONNECT: Client "::ffff:127.0.0.1"
Thu Apr 22 17:51:04 2021 [pid 75858] [www-ftp] OK LOGIN: Client "::ffff:127.0.0.1"
Thu Apr 22 17:51:04 2021 [pid 75860] [www-ftp] OK DELETE: Client "::ffff:127.0.0.1", "/var/www/html/libraries/vendor/paragonie/sodium_compat/src/Core32/Curve25519/README.md"
Thu Apr 22 17:51:04 2021 [pid 75860] [www-ftp] OK DELETE: Client "::ffff:127.0.0.1", "/var/www/html/libraries/vendor/typo3/phar-stream-wrapper/composer.json"

@richard67
Copy link
Member

It will need to make a PR for George's PR here #25559 to add the files handled in the changes of this PR to the list of files to be kept. Otherwise they will be added again when we run the script next time. I will care for that.

@brianteeman
Copy link
Contributor

I am sure they were removed for a specific reason. Its really not necesssary to distribute every file in a vendor folder. For example anything that goes through one of our build scripts such as tinymce results in (sometimes) a subset of files

@wilsonge
Copy link
Contributor

There are specific files in library/vendor that do get removed by the build script https://github.com/joomla/joomla-cms/blob/4.0-dev/build/build.php#L78-L127 - obviously however the aim is consistency :)

https://github.com/joomla/joomla-cms/blob/4.0-dev/build/build.php#L134-L149 I think should clear out all the composer.json files in a prod build. The readme.md file probably should be cleared out by this https://github.com/joomla/joomla-cms/blob/4.0-dev/build/build.php#L71 - I'm not 100% though - need to build up a prod style build this evening to be sure. Either way clearly the file check should be consistent with the nightly/prod build script - so sounds like there's an issue to be solved.

@wilsonge
Copy link
Contributor

wilsonge commented Jul 5, 2021

@PhilETaylor @richard67 is this still an issue? I know there was a lot of cleanup on the deleted folders list.

@richard67
Copy link
Member

@wilsonge Haven't you told me a few days ago in Glip that libraries/vendor/bin can safely be deleted because all libraries/vendor is handled by composer and so we don`t expect any user stuff there? If that is true, then the same applies to the files handled by this PR here.

Another open issue is that someone (Phil?) has replaced the calls to PHP functions to calls to the Folder class from the filesystem library so when a folder is deleted, all files and folders below it will be deleted, too, and so that files and folders lists would not make sense like they are now. I think this change has to be reverted and the script.php should use the standard PHP functions like it was before. But that's another issue which was discussed elsewhere (I think in one of my PR's for updating script.php).

@brianteeman
Copy link
Contributor

@wilsonge Haven't you told me a few days ago in Glip that libraries/vendor/bin can safely be deleted because all libraries/vendor is handled by composer and so we don`t expect any user stuff there? If that is true, then the same applies to the files handled by this PR here.

That is true on new installs only

@richard67
Copy link
Member

@wilsonge Haven't you told me a few days ago in Glip that libraries/vendor/bin can safely be deleted because all libraries/vendor is handled by composer and so we don`t expect any user stuff there? If that is true, then the same applies to the files handled by this PR here.

That is true on new installs only

@wilsonge Please clarify with respect to your comment here: wilsonge#69 (comment) . I am confused now.

@wilsonge
Copy link
Contributor

wilsonge commented Jul 6, 2021

That is true on new installs only

We don't support people doing custom composer actions and never have (this is why we don't ship with the composer.lock file). People doing custom composer stuff should be shipping their own vendor directories.

@PhilETaylor

This comment was marked as abuse.

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.

5 participants