diff --git a/lib/common/functions.sh b/lib/common/functions.sh index a7c412f4cb..55df896c46 100755 --- a/lib/common/functions.sh +++ b/lib/common/functions.sh @@ -10606,13 +10606,38 @@ configureHttpd() { # FOG_SCHEMA_INSTALL_TOKEN, sitting readable in the web root while a # different file is the one actually being used. Nothing reads it, # nothing reports it, and it survives every future upgrade. + # THE DISCOVERY EXTENSIONS ARE NOT OPTIONAL HERE, AND THEY ARE + # WORSE THAN THE .class.php CASE ABOVE. + # + # GH-1528 retired 52 more files the same way: every core page, + # hook, report and event moved from lib/{pages,hooks,reports, + # events}/..php to src//.php. A + # loop matching only *.class.php leaves all 52 behind. + # + # A stale .class.php is inert -- the comment above says why. A + # stale *.report.php is NOT. ReportManagement::loadCustomReports() + # merges core's src/Reports with the fileitems() walk that finds + # plugin reports, and the walk reaches lib/reports/ as well, so + # every core report is found TWICE and the Reports menu renders + # each of them twice. Measured on a 1.6 install upgraded this way: + # 17 reports became 30, 13 of them duplicates. + # + # So the sweep asks for the four discovery extensions alongside + # the class files. The keep-if-still-shipped test is unchanged and + # is what makes that safe: lib/router/ still ships its .class.php + # files and they are matched, tested and kept, exactly as before. dots "Removing retired class files from the old web folder" local relpath while IFS= read -r -d '' i; do relpath="${i#$webdirdest/}" [[ -e ${webdirsrc}/${relpath} ]] && continue rm -f "$i" >>$error_log 2>&1 - done < <(find "$webdirdest/lib" -maxdepth 2 -type f -name '*.class.php' -print0 2>>$error_log) + done < <(find "$webdirdest/lib" -maxdepth 2 -type f \( \ + -name '*.class.php' -o \ + -name '*.page.php' -o \ + -name '*.hook.php' -o \ + -name '*.report.php' -o \ + -name '*.event.php' \) -print0 2>>$error_log) errorStat $? fi fi diff --git a/tests/oldcopy-retires-moved-classes.test.sh b/tests/oldcopy-retires-moved-classes.test.sh index 012193a638..857d62aa80 100644 --- a/tests/oldcopy-retires-moved-classes.test.sh +++ b/tests/oldcopy-retires-moved-classes.test.sh @@ -79,10 +79,15 @@ fi # --------------------------------------------------------------------------- # The loop itself, lifted out of configureHttpd(). # --------------------------------------------------------------------------- +# Terminated on -print0 rather than on `done < <(find `: the find grew a +# multi-line \( -name ... -o -name ... \) list in GH-1531, and stopping at the +# first line would cut the snippet mid-command -- which fails as a SYNTAX +# ERROR in the eval below rather than as a wrong answer, but is exactly the +# kind of extraction that silently stops covering what it names. snippet=$(awk ' /^ local relpath$/ { grab = 1 } grab { print } - grab && /^ done < <\(find / { exit } + grab && /-print0 2>>\$error_log\)/ { exit } ' "$functions") if [[ -z $snippet ]]; then @@ -133,6 +138,29 @@ echo generated > "$webdirdest/commons/config.class.php" # A bundled plugin's own class file, one level deeper. echo plugin > "$webdirdest/lib/plugins/site/class/site.class.php" +# GH-1528 retired the four DISCOVERY extensions the same way: every core page, +# hook, report and event is src//.php now. These matter more +# than the class files above, which are inert once src/ answers first -- a +# stale *.report.php is FOUND by ReportManagement::loadCustomReports()'s +# plugin walk, so every core report appears in the menu twice. Reproduced on a +# 1.6 install: 17 reports became 30. +mkdir -p "$webdirdest/lib/pages" "$webdirdest/lib/hooks" \ + "$webdirdest/lib/reports" "$webdirdest/lib/events" \ + "$webdirsrc/lib/pages" "$webdirdest/lib/plugins/site/reports" +echo stale > "$webdirdest/lib/pages/hostmanagement.page.php" +echo stale > "$webdirdest/lib/hooks/bootitem.hook.php" +echo stale > "$webdirdest/lib/reports/audit_report.report.php" +echo stale > "$webdirdest/lib/events/hostlist.event.php" +# Still shipped under lib/, so the keep-if-present test must spare it. There is +# no such core file today, which is precisely why one is invented here: the +# obvious wrong fix deletes every discovery-extension file it finds. +: > "$webdirsrc/lib/pages/index.page.php" +echo kept > "$webdirdest/lib/pages/index.page.php" +# A bundled plugin's report, one level deeper than this loop's maxdepth. It +# belongs to the fog-plugins release, and deleting it would strip a report +# from an upgraded server. +echo plugin > "$webdirdest/lib/plugins/site/reports/site_report.report.php" + # Wrapped in a function, which is where it really runs (configureHttpd) and # what makes its `local` declaration legal. eval "fog_retire_class_files() { @@ -160,6 +188,18 @@ check "the generated commons/config.class.php survives" \ check "a bundled plugin's class file is not this loop's to delete" \ "$([[ -s $webdirdest/lib/plugins/site/class/site.class.php ]]; echo $?)" +check "a retired page/hook/report/event file is removed (GH-1528)" \ + "$([[ ! -e $webdirdest/lib/pages/hostmanagement.page.php \ + && ! -e $webdirdest/lib/hooks/bootitem.hook.php \ + && ! -e $webdirdest/lib/reports/audit_report.report.php \ + && ! -e $webdirdest/lib/events/hostlist.event.php ]]; echo $?)" + +check "a discovery-extension file the release still ships is left alone" \ + "$([[ -s $webdirdest/lib/pages/index.page.php ]]; echo $?)" + +check "a bundled plugin's report is not this loop's to delete" \ + "$([[ -s $webdirdest/lib/plugins/site/reports/site_report.report.php ]]; echo $?)" + # --------------------------------------------------------------------------- # The loop has to run on the RESTORED tree, before the new files are laid over # it. Run it after, and every retired file it was meant to delete has already