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

ci(checkers): Fix autoloader (re)generation for extraneous apps #37476

Merged

Conversation

ChristophWurst
Copy link
Member

  • Resolves: #

Summary

Running ./build/autoloaderchecker.sh should only touch apps that are part of this repo.

TODO

  • Fix the script

Checklist

@ChristophWurst ChristophWurst added this to the Nextcloud 27 milestone Mar 30, 2023
@ChristophWurst ChristophWurst self-assigned this Mar 30, 2023
@ChristophWurst ChristophWurst changed the title ci(checkers): Fix autoloader regeneration for extraneous apps ci(checkers): Fix autoloader (re)generation for extraneous apps Mar 30, 2023
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any chance to make it stop creating a huge diff when there are no change in files?

build/autoloaderchecker.sh Outdated Show resolved Hide resolved
@ChristophWurst
Copy link
Member Author

Any chance to make it stop creating a huge diff when there are no change in files?

Composer files are not designed to be commited to git. I can't think of a way to disable any diffs for the generated autoloader code.

If we remove the autoloaders from git and generate them at dev and release time we circumvent the problem.

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Apr 3, 2023
@ChristophWurst ChristophWurst force-pushed the ci/checkers/fix-autoloader-generation-extraneous-apps branch from 10a07c6 to b2c9a57 Compare April 3, 2023 17:59
@@ -22,6 +22,11 @@ echo "Regenerating main autoloader"
$COMPOSER_COMMAND dump-autoload -d $REPODIR

for app in ${REPODIR}/apps/*; do
if git check-ignore ${app} -q ; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be the other way arround? git check-ignore returns 0 if the file is ignored. This will only non ignored files will be skipped like this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if git check-ignore apps/mail -q ; then echo "mail should be skipped"; fi -> goes into the conditional branch. This is what we want here. If the file is ignored we want to continue the loop.

@nickvergessen
Copy link
Member

Btw this could also replace my local patch for build/image-optimization.sh right:

diff --git a/build/image-optimization.sh b/build/image-optimization.sh
index e559d8552f6..99ac9048e1a 100755
--- a/build/image-optimization.sh
+++ b/build/image-optimization.sh
@@ -33,6 +33,22 @@ function recursive_optimize_images() {
                return
        elif [[ "$DIR_NAME" == "tests" ]]; then
                return
+       elif [[ "$DIR_NAME" == "appsmine" ]]; then
+               return
+       elif [[ "$DIR_NAME" == "appscommunity" ]]; then
+               return
+       elif [[ "$DIR_NAME" == "appsmarketing" ]]; then
+               return
+       elif [[ "$DIR_NAME" == "appspackaged" ]]; then
+               return
+       elif [[ "$DIR_NAME" == "appssubscriptions" ]]; then
+               return
+       elif [[ "$DIR_NAME" == "appssupported" ]]; then
+               return
+       elif [[ "$DIR_NAME" == "appszips" ]]; then
+               return
+       elif [[ "$DIR_NAME" == "apps-action-templates" ]]; then
+               return
        fi
 
        # Optimize all PNGs

But I guess you don't want to add that here, do you?

@ChristophWurst
Copy link
Member Author

Sounds like a neat topic for another PR :)

@blizzz blizzz merged commit a578a5e into master Apr 14, 2023
@blizzz blizzz deleted the ci/checkers/fix-autoloader-generation-extraneous-apps branch April 14, 2023 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish technical debt
Projects
Development

Successfully merging this pull request may close these issues.

6 participants