Skip to content

Swift: teach autobuilder about SPM, CocoaPods, and Carthage #13979

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

Merged
merged 6 commits into from
Aug 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"markdownMessage": "`autobuild` failed to run the detected build command:\n\n```\n/usr/bin/xcodebuild build -project <test-root-directory>/hello-failure.xcodeproj -target hello-failure CODE_SIGNING_REQUIRED=NO CODE_SIGNING_ALLOWED=NO\n```\n\nSet up a [manual build command][1] or [check the logs of the autobuild step][2].\n\n[1]: https://docs.github.com/en/enterprise-server/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-the-codeql-workflow-for-compiled-languages#adding-build-steps-for-a-compiled-language\n[2]: https://docs.github.com/en/actions/monitoring-and-troubleshooting-workflows/using-workflow-run-logs",
"markdownMessage": "`autobuild` failed to run the build command:\n\n```\n/usr/bin/xcodebuild build -project <test-root-directory>/hello-failure.xcodeproj -target hello-failure CODE_SIGNING_REQUIRED=NO CODE_SIGNING_ALLOWED=NO\n```\n\nSet up a [manual build command][1] or [check the logs of the autobuild step][2].\n\n[1]: https://docs.github.com/en/enterprise-server/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-the-codeql-workflow-for-compiled-languages#adding-build-steps-for-a-compiled-language\n[2]: https://docs.github.com/en/actions/monitoring-and-troubleshooting-workflows/using-workflow-run-logs",
"severity": "error",
"source": {
"extractorName": "swift",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"markdownMessage": "`autobuild` could not detect an Xcode project or workspace.\n\nSet up a [manual build command][1].\n\n[1]: https://docs.github.com/en/enterprise-server/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-the-codeql-workflow-for-compiled-languages#adding-build-steps-for-a-compiled-language",
"markdownMessage": "`autobuild` detected neither an Xcode project or workspace, nor a Swift package\n\nSet up a [manual build command][1].\n\n[1]: https://docs.github.com/en/enterprise-server/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-the-codeql-workflow-for-compiled-languages#adding-build-steps-for-a-compiled-language",
"severity": "error",
"source": {
"extractorName": "swift",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{
"markdownMessage": "A Swift package was detected, but no viable Xcode target was found.\n\nSwift Package Manager builds are not currently supported by `autobuild`. Set up a [manual build command][1].\n\n[1]: https://docs.github.com/en/enterprise-server/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-the-codeql-workflow-for-compiled-languages#adding-build-steps-for-a-compiled-language",
"markdownMessage": "`autobuild` failed to run the build command:\n\n```\n/usr/bin/swift build --package-path <test-root-directory>/hello-objective\n```\n\nSet up a [manual build command][1] or [check the logs of the autobuild step][2].\n\n[1]: https://docs.github.com/en/enterprise-server/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-the-codeql-workflow-for-compiled-languages#adding-build-steps-for-a-compiled-language\n[2]: https://docs.github.com/en/actions/monitoring-and-troubleshooting-workflows/using-workflow-run-logs",
"severity": "error",
"source": {
"extractorName": "swift",
"id": "swift/autobuilder/spm-not-supported",
"name": "Swift Package Manager is not supported"
"id": "swift/autobuilder/build-command-failed",
"name": "Detected build command failed"
},
"visibility": {
"cliSummaryTable": true,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{
"markdownMessage": "A Swift package was detected, but no viable Xcode target was found.\n\nSwift Package Manager builds are not currently supported by `autobuild`. Set up a [manual build command][1].\n\n[1]: https://docs.github.com/en/enterprise-server/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-the-codeql-workflow-for-compiled-languages#adding-build-steps-for-a-compiled-language",
"markdownMessage": "`autobuild` failed to run the build command:\n\n```\n/usr/bin/swift build --package-path <test-root-directory>\n```\n\nSet up a [manual build command][1] or [check the logs of the autobuild step][2].\n\n[1]: https://docs.github.com/en/enterprise-server/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-the-codeql-workflow-for-compiled-languages#adding-build-steps-for-a-compiled-language\n[2]: https://docs.github.com/en/actions/monitoring-and-troubleshooting-workflows/using-workflow-run-logs",
"severity": "error",
"source": {
"extractorName": "swift",
"id": "swift/autobuilder/spm-not-supported",
"name": "Swift Package Manager is not supported"
"id": "swift/autobuilder/build-command-failed",
"name": "Detected build command failed"
},
"visibility": {
"cliSummaryTable": true,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{
"markdownMessage": "A Swift package was detected, but no viable Xcode target was found.\n\nSwift Package Manager builds are not currently supported by `autobuild`. Set up a [manual build command][1].\n\n[1]: https://docs.github.com/en/enterprise-server/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-the-codeql-workflow-for-compiled-languages#adding-build-steps-for-a-compiled-language",
"markdownMessage": "`autobuild` failed to run the build command:\n\n```\n/usr/bin/swift build --package-path <test-root-directory>\n```\n\nSet up a [manual build command][1] or [check the logs of the autobuild step][2].\n\n[1]: https://docs.github.com/en/enterprise-server/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-the-codeql-workflow-for-compiled-languages#adding-build-steps-for-a-compiled-language\n[2]: https://docs.github.com/en/actions/monitoring-and-troubleshooting-workflows/using-workflow-run-logs",
"severity": "error",
"source": {
"extractorName": "swift",
"id": "swift/autobuilder/spm-not-supported",
"name": "Swift Package Manager is not supported"
"id": "swift/autobuilder/build-command-failed",
"name": "Detected build command failed"
},
"visibility": {
"cliSummaryTable": true,
Expand Down
2 changes: 2 additions & 0 deletions swift/tools/autobuild.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#!/bin/bash

if [[ "$OSTYPE" == "darwin"* ]]; then
export CODEQL_SWIFT_CARTHAGE_EXEC=`which carthage`
export CODEQL_SWIFT_POD_EXEC=`which pod`
exec "${CODEQL_EXTRACTOR_SWIFT_ROOT}/tools/${CODEQL_PLATFORM}/xcode-autobuilder"
else
exec "${CODEQL_EXTRACTOR_SWIFT_ROOT}/tools/${CODEQL_PLATFORM}/autobuilder-incompatible-os"
Expand Down
58 changes: 48 additions & 10 deletions swift/xcode-autobuilder/XcodeBuildRunner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,21 @@ static bool exec(const std::vector<std::string>& argv) {
return true;
}

void buildTarget(Target& target, bool dryRun) {
static bool run_build_command(const std::vector<std::string>& argv, bool dryRun) {
if (dryRun) {
std::cout << absl::StrJoin(argv, " ") << "\n";
} else {
if (!exec(argv)) {
DIAGNOSE_ERROR(buildCommandFailed,
"`autobuild` failed to run the build command:\n\n```\n{}\n```",
absl::StrJoin(argv, " "));
return false;
}
}
return true;
}

bool buildXcodeTarget(const XcodeTarget& target, bool dryRun) {
std::vector<std::string> argv({"/usr/bin/xcodebuild", "build"});
if (!target.workspace.empty()) {
argv.push_back("-workspace");
Expand All @@ -65,16 +79,40 @@ void buildTarget(Target& target, bool dryRun) {
argv.push_back(target.name);
argv.push_back("CODE_SIGNING_REQUIRED=NO");
argv.push_back("CODE_SIGNING_ALLOWED=NO");
return run_build_command(argv, dryRun);
}

if (dryRun) {
std::cout << absl::StrJoin(argv, " ") << "\n";
} else {
if (!exec(argv)) {
DIAGNOSE_ERROR(buildCommandFailed,
"`autobuild` failed to run the detected build command:\n\n```\n{}\n```",
absl::StrJoin(argv, " "));
codeql::Log::flush();
exit(1);
bool buildSwiftPackage(const std::filesystem::path& packageFile, bool dryRun) {
std::vector<std::string> argv(
{"/usr/bin/swift", "build", "--package-path", packageFile.parent_path()});
return run_build_command(argv, dryRun);
}

static void pod_install(const std::string& pod, const std::filesystem::path& podfile, bool dryRun) {
std::vector<std::string> argv(
{pod, "install", "--project-directory=" + podfile.parent_path().string()});
run_build_command(argv, dryRun);
}

static void carthage_install(const std::string& carthage,
const std::filesystem::path& podfile,
bool dryRun) {
std::vector<std::string> argv(
{carthage, "bootstrap", "--project-directory", podfile.parent_path()});
run_build_command(argv, dryRun);
}

void installDependencies(const ProjectStructure& target, bool dryRun) {
auto pod = std::string(getenv("CODEQL_SWIFT_POD_EXEC") ?: "");
auto carthage = std::string(getenv("CODEQL_SWIFT_CARTHAGE_EXEC") ?: "");
Comment on lines +106 to +107
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL about this ?: GNU extension that I knew nothing about 🙂

As a nit, this can be made more standard conforming using std::string_view, which contrary to std::string accepts a nullptr (and treats it equivalent to ""). Or move the getenv as a nested if, for example

if (!target.podfiles.empty()) {
  if (auto pod = getenv("CODEQL_SWIFT_POD_EXEC")) {
    pod_install(pod, podfile, dryRun);
  }
}

or as one single if with initialization:

if (auto pod = getenv("CODEQL_SWIFT_POD_EXEC"); pod && !target.podfiles.empty()) {
  pod_install(pod, podfile, dryRun);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The first snippet is not exactly correct as we may have podfiles on linux, but won't have pod executable there.
And I don't find the second necessarily better than what we have now

Copy link
Contributor

Choose a reason for hiding this comment

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

as far as I understand, the first snippet is equivalent, as the pod installation will happen only if both the conditions are met. The conditions are just evaluated in swapped order (and the env variable won't be inspected if podfiles is empty).

What itches me here, is the use of a non-standard extension, that might also be surprising to some developers (it was a bit to me). On the other hand, we do enforce compilation with clang, so maybe the non-standardness is not such a big deal... It's just that I personally only use non-standard C++ stuff when there is no sensible alternative, which I think there is here.

But that's a nit, I'm also kinda ok with leaving this as is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is as non-standard as #pragma once 😄

if (!pod.empty() && !target.podfiles.empty()) {
for (auto& podfile : target.podfiles) {
pod_install(pod, podfile, dryRun);
}
}
if (!carthage.empty() && !target.cartfiles.empty()) {
for (auto& cartfile : target.cartfiles) {
carthage_install(carthage, cartfile, dryRun);
}
}
}
6 changes: 5 additions & 1 deletion swift/xcode-autobuilder/XcodeBuildRunner.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
#pragma once

#include "swift/xcode-autobuilder/XcodeTarget.h"
#include "swift/xcode-autobuilder/XcodeProjectParser.h"
#include <filesystem>

void buildTarget(Target& target, bool dryRun);
void installDependencies(const ProjectStructure& target, bool dryRun);
bool buildXcodeTarget(const XcodeTarget& target, bool dryRun);
bool buildSwiftPackage(const std::filesystem::path& packageFile, bool dryRun);
58 changes: 33 additions & 25 deletions swift/xcode-autobuilder/XcodeProjectParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -198,14 +198,30 @@ static std::unordered_map<std::string, TargetData> mapTargetsToWorkspace(
return targetMapping;
}

static std::vector<fs::path> collectFiles(const std::string& workingDir) {
struct ProjectFiles {
std::vector<fs::path> xcodeFiles;
std::vector<fs::path> packageFiles;
std::vector<fs::path> podfiles;
std::vector<fs::path> cartfiles;
};

static ProjectFiles scanWorkingDir(const std::string& workingDir) {
ProjectFiles structure;
fs::path workDir(workingDir);
std::vector<fs::path> files;
auto end = fs::recursive_directory_iterator();
for (auto it = fs::recursive_directory_iterator(workDir); it != end; ++it) {
const auto& p = it->path();
if (p.filename() == "Package.swift") {
files.push_back(p);
structure.packageFiles.push_back(p);
continue;
}
if (p.filename() == "Podfile") {
structure.podfiles.push_back(p);
continue;
}
if (p.filename() == "Cartfile" || p.filename() == "Cartfile.private") {
structure.cartfiles.push_back(p);
continue;
}
if (!it->is_directory()) {
Expand All @@ -217,43 +233,31 @@ static std::vector<fs::path> collectFiles(const std::string& workingDir) {
continue;
}
if (p.extension() == ".xcodeproj" || p.extension() == ".xcworkspace") {
files.push_back(p);
structure.xcodeFiles.push_back(p);
Copy link
Contributor

@redsun82 redsun82 Aug 21, 2023

Choose a reason for hiding this comment

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

maybe as you are anyway creating this structure with fields for the different kinds of files, you can also split .xcodeproj and .xcworkspace in separate xcodeProjects and xcodeWorkspaces fields of this structure, and avoid testing the extension again later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I thought about this but decided not to as these two go hand in hand and it doesn't make much difference if we do it the other way.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it'd be a tad cleaner, but I'll leave it to your judgement 🙂

}
}
return files;
return structure;
}

static std::unordered_map<std::string, std::vector<std::string>> collectWorkspaces(
const std::string& workingDir,
bool& swiftPackageEncountered) {
const ProjectFiles& projectFiles) {
// Here we are collecting list of all workspaces and Xcode projects corresponding to them
// Projects without workspaces go into the same "empty-workspace" bucket
swiftPackageEncountered = false;
std::unordered_map<std::string, std::vector<std::string>> workspaces;
std::unordered_set<std::string> projectsBelongingToWorkspace;
std::vector<fs::path> files = collectFiles(workingDir);
for (auto& path : files) {
for (auto& path : projectFiles.xcodeFiles) {
if (path.extension() == ".xcworkspace") {
auto projects = readProjectsFromWorkspace(path.string());
for (auto& project : projects) {
projectsBelongingToWorkspace.insert(project.string());
workspaces[path.string()].push_back(project.string());
}
} else if (!swiftPackageEncountered && path.filename() == "Package.swift") {
// a package manifest must begin with a specific header comment
// see https://docs.swift.org/package-manager/PackageDescription/PackageDescription.html
static constexpr std::string_view packageHeader = "// swift-tools-version:";
std::array<char, packageHeader.size()> buffer{};
std::string_view bufferView{buffer.data(), buffer.size()};
if (std::ifstream{path}.read(buffer.data(), buffer.size()) && bufferView == packageHeader) {
swiftPackageEncountered = true;
}
}
}
// Collect all projects not belonging to any workspace into a separate empty bucket
for (auto& path : files) {
for (auto& path : projectFiles.xcodeFiles) {
if (path.extension() == ".xcodeproj") {
if (projectsBelongingToWorkspace.count(path.string())) {
if (projectsBelongingToWorkspace.contains(path.string())) {
continue;
}
workspaces[std::string()].push_back(path.string());
Expand All @@ -262,11 +266,15 @@ static std::unordered_map<std::string, std::vector<std::string>> collectWorkspac
return workspaces;
}

Targets collectTargets(const std::string& workingDir) {
Targets ret;
ProjectStructure scanProjectStructure(const std::string& workingDir) {
ProjectStructure ret;
// Getting a list of workspaces and the project that belong to them
auto workspaces = collectWorkspaces(workingDir, ret.swiftPackageEncountered);
auto projectFiles = scanWorkingDir(workingDir);
auto workspaces = collectWorkspaces(projectFiles);
ret.xcodeEncountered = !workspaces.empty();
ret.swiftPackages = std::move(projectFiles.packageFiles);
ret.podfiles = std::move(projectFiles.podfiles);
ret.cartfiles = std::move(projectFiles.cartfiles);
if (!ret.xcodeEncountered) {
return ret;
}
Expand All @@ -278,8 +286,8 @@ Targets collectTargets(const std::string& workingDir) {
auto targetFilesMapping = mapTargetsToSourceFiles(workspaces);

for (auto& [targetName, data] : targetMapping) {
ret.targets.push_back(Target{data.workspace, data.project, targetName, data.type,
targetFilesMapping[targetName]});
ret.xcodeTargets.push_back(XcodeTarget{data.workspace, data.project, targetName, data.type,
targetFilesMapping[targetName]});
}
return ret;
}
14 changes: 10 additions & 4 deletions swift/xcode-autobuilder/XcodeProjectParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,17 @@
#include "swift/xcode-autobuilder/XcodeTarget.h"
#include <vector>
#include <string>
#include <filesystem>

struct Targets {
std::vector<Target> targets;
struct ProjectStructure {
std::vector<XcodeTarget> xcodeTargets;
bool xcodeEncountered;
bool swiftPackageEncountered;
// Swift Package Manager support
std::vector<std::filesystem::path> swiftPackages;
// CocoaPods support
std::vector<std::filesystem::path> podfiles;
// Carthage support
std::vector<std::filesystem::path> cartfiles;
};

Targets collectTargets(const std::string& workingDir);
ProjectStructure scanProjectStructure(const std::string& workingDir);
4 changes: 2 additions & 2 deletions swift/xcode-autobuilder/XcodeTarget.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
#include <string>
#include <binlog/adapt_struct.hpp>

struct Target {
struct XcodeTarget {
std::string workspace;
std::string project;
std::string name;
std::string type;
size_t fileCount;
};

BINLOG_ADAPT_STRUCT(Target, workspace, project, name, type, fileCount);
BINLOG_ADAPT_STRUCT(XcodeTarget, workspace, project, name, type, fileCount);
Loading