nextcloud / updater

:arrows_counterclockwise: The updater app to keep your Nextcloud up-to-date
GNU Affero General Public License v3.0
45 stars 31 forks source link

Also include *.config.php files #535

Closed JensSpanier closed 4 months ago

JensSpanier commented 5 months ago

Fixes #384

Inspired by this: https://github.com/nextcloud/server/blob/566586c609253f498c1b3f0c6c004b30ec2d998b/lib/private/Config.php#L205-L252

index.php and updater.phar still need to be regenerated.

github-actions[bot] commented 4 months ago

Hello there, Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

come-nc commented 4 months ago

Please fix DCO by signing off your commit. Also, use make index.php updater.phar to update them. psalm errors:

ERROR: UndefinedVariable - lib/Updater.php:62:10 - Cannot find referenced variable $CONFIG (see https://psalm.dev/024)
            unset($CONFIG);

ERROR: UnresolvableInclude - lib/Updater.php:65:4 - Cannot resolve the given expression to a file path (see https://psalm.dev/106)
            /** @var array $CONFIG */
            require_once $configFile;

ERROR: RedundantConditionGivenDocblockType - lib/Updater.php:67:8 - Docblock-defined type array<array-key, mixed> for $CONFIG is never null (see https://psalm.dev/156)
            if (isset($CONFIG) && is_array($CONFIG)) {

ERROR: RedundantConditionGivenDocblockType - lib/Updater.php:67:8 - Docblock-defined type array<array-key, mixed> for $CONFIG is always array (see https://psalm.dev/156)
            if (isset($CONFIG) && is_array($CONFIG)) {

ERROR: RedundantConditionGivenDocblockType - lib/Updater.php:67:26 - Docblock-defined type array<array-key, mixed> for $CONFIG is always array (see https://psalm.dev/156)
            if (isset($CONFIG) && is_array($CONFIG)) {

You should move the /** @var array $CONFIG */ inside the if I think. For the unset you may have so use psalm-suppress, or maybe do it at the end of the loop instead of the beginning.

JensSpanier commented 4 months ago

Can you please review again? Thanks!

come-nc commented 4 months ago

@JensSpanier There is a leading space on some lines that codesniffer is complaining about. Please run composer run cs:fix to fix it, then make index.php updater.phar to update them, and then push the result?

JensSpanier commented 4 months ago

There is a leading space on some lines that codesniffer is complaining about.

Sorry about that. Hope it looks good this time.

come-nc commented 4 months ago

@JensSpanier This is causing troubles in some cases as explained in https://github.com/nextcloud/server/issues/44157

Is that something that you had the need for yourself? I’m not sure what is the best fix, we can catch Throwables from the require I suppose but we cannot log the error I think. I do not think it’s possible to provide the OC class at this point.

JensSpanier commented 4 months ago

@come-nc I wasn't aware that the OC class can appear in configuration files. I'm having the same problem like in #384. I'm also hosting at the provider netcup. I think it would be enough if you could specify the config files that will be read in when calling updater.phar. Or just another way to overwrite the config parameter datadirectory when calling updater.phar.

I also think we should reopen #384 because it's not fixed anymore.