Skip to content

Commit 959e54b

Browse files
williamjallenwilliamjallen
authored
Clean up legacy build error and build failure models (#3433)
Incremental progress towards the eventual combination of these models. The last significant difference between these models is the build failure arguments, which I plan to address in a follow-up PR.
1 parent 24f6a30 commit 959e54b

File tree

9 files changed

+231
-372
lines changed

9 files changed

+231
-372
lines changed

‎app/Http/Controllers/BuildController.php‎

Lines changed: 119 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -928,14 +928,14 @@ public function apiViewBuildError(): JsonResponse
928928
$resolvedBuildErrors = $this->build->GetResolvedBuildErrors($type);
929929
if ($resolvedBuildErrors !== false) {
930930
while ($resolvedBuildError = $resolvedBuildErrors->fetch()) {
931-
$this->addErrorResponse(BuildError::marshal($resolvedBuildError, $this->project, $revision), $response);
931+
$this->addErrorResponse(self::marshalBuildError($resolvedBuildError, $this->project, $revision), $response);
932932
}
933933
}
934934

935935
// Build failure table
936936
$resolvedBuildFailures = $this->build->GetResolvedBuildFailures($type);
937937
foreach ($resolvedBuildFailures as $resolvedBuildFailure) {
938-
$marshaledResolvedBuildFailure = BuildFailure::marshal($resolvedBuildFailure, $this->project, $revision, false, $buildfailure);
938+
$marshaledResolvedBuildFailure = self::marshalBuildFailure($resolvedBuildFailure, $this->project, $revision, false, $buildfailure);
939939

940940
if ($this->project->DisplayLabels) {
941941
get_labels_JSON_from_query_results('
@@ -968,14 +968,14 @@ public function apiViewBuildError(): JsonResponse
968968
$buildErrors = $this->build->GetErrors($filter_error_properties);
969969

970970
foreach ($buildErrors as $error) {
971-
$this->addErrorResponse(BuildError::marshal($error, $this->project, $revision), $response);
971+
$this->addErrorResponse(self::marshalBuildError($error, $this->project, $revision), $response);
972972
}
973973

974974
// Build failure table
975975
$buildFailures = $this->build->GetFailures(['type' => $type]);
976976

977977
foreach ($buildFailures as $fail) {
978-
$failure = BuildFailure::marshal($fail, $this->project, $revision, true, $buildfailure);
978+
$failure = self::marshalBuildFailure($fail, $this->project, $revision, true, $buildfailure);
979979

980980
if ($this->project->DisplayLabels) {
981981
$labels = RichBuildAlert::findOrFail((int) $fail['id'])->labels()->pluck('text');
@@ -995,6 +995,121 @@ public function apiViewBuildError(): JsonResponse
995995
return response()->json(cast_data_for_JSON($response));
996996
}
997997

998+
/**
999+
* @return array{
1000+
* 'file': string,
1001+
* 'directory': string,
1002+
* }
1003+
*/
1004+
private static function GetSourceFileFromBuildError(array $data): array
1005+
{
1006+
$sourceFile = [];
1007+
1008+
// Detect if the source directory has already been replaced by CTest
1009+
// with /.../. If so, sourcefile is already a relative path from the
1010+
// root of the source tree.
1011+
if (str_contains($data['stdoutput'], '/.../')) {
1012+
$parts = explode('/', $data['sourcefile']);
1013+
$sourceFile['file'] = array_pop($parts);
1014+
$sourceFile['directory'] = implode('/', $parts);
1015+
} else {
1016+
$sourceFile['file'] = basename($data['sourcefile']);
1017+
$sourceFile['directory'] = dirname($data['sourcefile']);
1018+
}
1019+
1020+
return $sourceFile;
1021+
}
1022+
1023+
// Ideally $data would be loaded into $this
1024+
// need an id field?
1025+
/**
1026+
* Marshals the data of a particular build error into a serializable
1027+
* friendly format.
1028+
*
1029+
* Requires the $data of a build error, the $project, and the buildupdate.revision.
1030+
*
1031+
* @return array<string,mixed>
1032+
**/
1033+
public static function marshalBuildError(array $data, \CDash\Model\Project $project, $revision): array
1034+
{
1035+
deepEncodeHTMLEntities($data);
1036+
1037+
$sourceFile = self::GetSourceFileFromBuildError($data);
1038+
1039+
// When building without launchers, CTest truncates the source dir to
1040+
// /.../<project-name>/. Use this pattern to linkify compiler output.
1041+
$source_dir = "/\.\.\./[^/]+";
1042+
1043+
$marshaled = [
1044+
'new' => (int) ($data['newstatus'] ?? -1),
1045+
'logline' => (int) $data['logline'],
1046+
'cvsurl' => RepositoryUtils::get_diff_url($project->Id, $project->CvsUrl ?? '', $sourceFile['directory'], $sourceFile['file'], $revision),
1047+
'precontext' => '',
1048+
'text' => RepositoryUtils::linkify_compiler_output($project->CvsUrl ?? '', $source_dir, $revision, $data['stdoutput']),
1049+
'postcontext' => '',
1050+
'sourcefile' => $data['sourcefile'],
1051+
'sourceline' => $data['sourceline'],
1052+
];
1053+
1054+
if (isset($data['subprojectid'])) {
1055+
$marshaled['subprojectid'] = $data['subprojectid'];
1056+
$marshaled['subprojectname'] = $data['subprojectname'];
1057+
}
1058+
1059+
return $marshaled;
1060+
}
1061+
1062+
/**
1063+
* Marshal a build failure, this includes the build failure arguments.
1064+
**/
1065+
public static function marshalBuildFailure($data, \CDash\Model\Project $project, $revision, $linkifyOutput, $buildfailure): array
1066+
{
1067+
deepEncodeHTMLEntities($data);
1068+
1069+
$marshaled = array_merge([
1070+
'language' => $data['language'],
1071+
'sourcefile' => $data['sourcefile'],
1072+
'targetname' => $data['targetname'],
1073+
'outputfile' => $data['outputfile'],
1074+
'outputtype' => $data['outputtype'],
1075+
'workingdirectory' => $data['workingdirectory'],
1076+
'exitcondition' => $data['exitcondition'],
1077+
], $buildfailure->GetBuildFailureArguments((int) $data['id']));
1078+
1079+
$marshaled['stderror'] = $data['stderror'];
1080+
$marshaled['stdoutput'] = $data['stdoutput'];
1081+
1082+
if (isset($data['sourcefile'])) {
1083+
$file = basename($data['sourcefile']);
1084+
$directory = dirname($data['sourcefile']);
1085+
1086+
$source_dir = RepositoryUtils::get_source_dir($project->Id, $project->CvsUrl ?? '', $directory);
1087+
if (str_starts_with($directory, $source_dir)) {
1088+
$directory = substr($directory, strlen($source_dir));
1089+
}
1090+
1091+
$marshaled['cvsurl'] = RepositoryUtils::get_diff_url($project->Id,
1092+
$project->CvsUrl ?? '',
1093+
$directory,
1094+
$file,
1095+
$revision);
1096+
1097+
if ($source_dir !== null && $linkifyOutput) {
1098+
$marshaled['stderror'] = RepositoryUtils::linkify_compiler_output($project->CvsUrl ?? '', $source_dir,
1099+
$revision, $data['stderror']);
1100+
$marshaled['stdoutput'] = RepositoryUtils::linkify_compiler_output($project->CvsUrl ?? '', $source_dir,
1101+
$revision, $data['stdoutput']);
1102+
}
1103+
}
1104+
1105+
if (isset($data['subprojectid'])) {
1106+
$marshaled['subprojectid'] = $data['subprojectid'];
1107+
$marshaled['subprojectname'] = $data['subprojectname'];
1108+
}
1109+
1110+
return $marshaled;
1111+
}
1112+
9981113
public function manageBuildGroup(): View
9991114
{
10001115
$this->setProjectById(request()->integer('projectid'));

‎app/cdash/app/Model/Build.php‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -609,9 +609,28 @@ public function GetFailures(array $propertyFilters = [], int $fetchStyle = PDO::
609609
if ($this->IsParentBuild()) {
610610
$failures = $this->GetFailuresForChildren($fetchStyle);
611611
} else {
612-
$buildFailure = new BuildFailure();
613-
$buildFailure->BuildId = $this->Id;
614-
$failures = $buildFailure->GetFailuresForBuild($fetchStyle);
612+
$sql = '
613+
SELECT
614+
bf.id,
615+
bf.buildid,
616+
bf.workingdirectory,
617+
bf.sourcefile,
618+
bf.newstatus,
619+
bf.stdoutput,
620+
bf.stderror,
621+
bf.type,
622+
bf.exitcondition,
623+
bf.language,
624+
bf.targetname,
625+
bf.outputfile,
626+
bf.outputtype
627+
FROM buildfailure AS bf
628+
WHERE bf.buildid=?
629+
ORDER BY bf.id
630+
';
631+
$query = $this->PDO->prepare($sql);
632+
pdo_execute($query, [$this->Id]);
633+
$failures = $query->fetchAll($fetchStyle);
615634
}
616635

617636
if ($failures !== false) {

‎app/cdash/app/Model/BuildError.php‎

Lines changed: 0 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
namespace CDash\Model;
1919

2020
use App\Models\BasicBuildAlert;
21-
use App\Utils\RepositoryUtils;
2221

2322
/** BuildError */
2423
class BuildError
@@ -60,70 +59,6 @@ public function Insert(): void
6059
]);
6160
}
6261

63-
/**
64-
* @return array{
65-
* 'file': string,
66-
* 'directory': string,
67-
* }
68-
*/
69-
private static function GetSourceFile(array $data): array
70-
{
71-
$sourceFile = [];
72-
73-
// Detect if the source directory has already been replaced by CTest
74-
// with /.../. If so, sourcefile is already a relative path from the
75-
// root of the source tree.
76-
if (str_contains($data['stdoutput'], '/.../')) {
77-
$parts = explode('/', $data['sourcefile']);
78-
$sourceFile['file'] = array_pop($parts);
79-
$sourceFile['directory'] = implode('/', $parts);
80-
} else {
81-
$sourceFile['file'] = basename($data['sourcefile']);
82-
$sourceFile['directory'] = dirname($data['sourcefile']);
83-
}
84-
85-
return $sourceFile;
86-
}
87-
88-
// Ideally $data would be loaded into $this
89-
// need an id field?
90-
/**
91-
* Marshals the data of a particular build error into a serializable
92-
* friendly format.
93-
*
94-
* Requires the $data of a build error, the $project, and the buildupdate.revision.
95-
*
96-
* @return array<string,mixed>
97-
**/
98-
public static function marshal(array $data, Project $project, $revision): array
99-
{
100-
deepEncodeHTMLEntities($data);
101-
102-
$sourceFile = self::GetSourceFile($data);
103-
104-
// When building without launchers, CTest truncates the source dir to
105-
// /.../<project-name>/. Use this pattern to linkify compiler output.
106-
$source_dir = "/\.\.\./[^/]+";
107-
108-
$marshaled = [
109-
'new' => (int) ($data['newstatus'] ?? -1),
110-
'logline' => (int) $data['logline'],
111-
'cvsurl' => RepositoryUtils::get_diff_url($project->Id, $project->CvsUrl, $sourceFile['directory'], $sourceFile['file'], $revision),
112-
'precontext' => '',
113-
'text' => RepositoryUtils::linkify_compiler_output($project->CvsUrl ?? '', $source_dir, $revision, $data['stdoutput']),
114-
'postcontext' => '',
115-
'sourcefile' => $data['sourcefile'],
116-
'sourceline' => $data['sourceline'],
117-
];
118-
119-
if (isset($data['subprojectid'])) {
120-
$marshaled['subprojectid'] = $data['subprojectid'];
121-
$marshaled['subprojectname'] = $data['subprojectname'];
122-
}
123-
124-
return $marshaled;
125-
}
126-
12762
/**
12863
* Returns a self referencing URI for the current BuildError.
12964
*/

0 commit comments

Comments
 (0)