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

[4] Dont delete these files from libraries/vendor #33250

Closed
wants to merge 1 commit into from
Closed

[4] Dont delete these files from libraries/vendor #33250

wants to merge 1 commit into from

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.

None yet

5 participants