Skip to content

Comments

Also include *.config.php files#535

Merged
come-nc merged 5 commits intonextcloud:masterfrom
JensSpanier:master
Mar 6, 2024
Merged

Also include *.config.php files#535
come-nc merged 5 commits intonextcloud:masterfrom
JensSpanier:master

Conversation

@JensSpanier
Copy link
Contributor

@github-actions

This comment was marked as outdated.

@come-nc
Copy link
Collaborator

come-nc commented Feb 27, 2024

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.

@solracsf solracsf removed their request for review February 27, 2024 11:26
JensSpanier and others added 3 commits March 5, 2024 08:12
@JensSpanier
Copy link
Contributor Author

Can you please review again? Thanks!

@come-nc
Copy link
Collaborator

come-nc commented Mar 5, 2024

@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?

Signed-off-by: Jens Spanier <[email protected]>
@JensSpanier
Copy link
Contributor Author

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

Sorry about that. Hope it looks good this time.

@come-nc
Copy link
Collaborator

come-nc commented Mar 14, 2024

@JensSpanier This is causing troubles in some cases as explained in nextcloud/server#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
Copy link
Contributor Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Updater ignores additional config.php files

4 participants