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

Bazel: various fixes #9243

Merged
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
Expand Up @@ -40,6 +40,11 @@ internal val JSON = Json {
* A Bazel registry which is either local or remote.
*/
interface BazelModuleRegistryService {
/**
* The URLs of this registry service.
*/
val urls: List<String>

/**
* Retrieve the metadata for a module.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,10 @@
const val SOURCE_JSON = "source.json"

/**
* A Bazel registry which is located on the local file system.
* A Bazel registry for the workspace [directory] which is located on the local file system. It is configured as [url]

Check warning on line 36 in clients/bazel-module-registry/src/main/kotlin/LocalBazelModuleRegistryService.kt

View workflow job for this annotation

GitHub Actions / qodana-scan

Unresolved reference in KDoc

Cannot resolve symbol 'url'

Check warning on line 36 in clients/bazel-module-registry/src/main/kotlin/LocalBazelModuleRegistryService.kt

View workflow job for this annotation

GitHub Actions / qodana-scan

Unresolved reference in KDoc

Cannot resolve symbol 'directory'
Dismissed Show dismissed Hide dismissed
Dismissed Show dismissed Hide dismissed
* in the '.bazelrc' file.
*/
class LocalBazelModuleRegistryService(directory: File) : BazelModuleRegistryService {
class LocalBazelModuleRegistryService(url: String, directory: File) : BazelModuleRegistryService {
companion object {
/** A prefix for URLs pointing to local files. */
private const val FILE_URL_PREFIX = "file://"
Expand All @@ -53,7 +54,7 @@
.replace(WORKSPACE_PLACEHOLDER, projectDir.absolutePath)

logger.info { "Creating local Bazel module registry at '$directory'." }
LocalBazelModuleRegistryService(File(directory))
LocalBazelModuleRegistryService(url, File(directory))
}
}

Expand All @@ -74,6 +75,8 @@
moduleDirectory = registryFile.resolveSibling(BAZEL_MODULES_DIR)
}

override val urls: List<String> = listOf(url)

override suspend fun getModuleMetadata(name: String): ModuleMetadata {
val metadataJson = moduleDirectory.resolve(name).resolve(METADATA_JSON)
require(metadataJson.isFile)
Expand Down
1 change: 1 addition & 0 deletions clients/bazel-module-registry/src/main/kotlin/Model.kt
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ data class ModuleMetadata(
@Serializable
data class Maintainer(
val email: String? = null,
val github: String? = null,
sschuberth marked this conversation as resolved.
Show resolved Hide resolved
val name: String? = null
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,21 @@

import retrofit2.Retrofit
import retrofit2.converter.kotlinx.serialization.asConverterFactory
import retrofit2.http.GET
import retrofit2.http.Path

/**
* The client uses the Bazel Central Registry by default.
*/
const val DEFAULT_URL = "https://bcr.bazel.build"

/**
* Interface for a Bazel Module Registry, based on the directory structure of https://bcr.bazel.build/ and the Git
* repository it is based on (https://github.com/bazelbuild/bazel-central-registry/).
* A remote Bazel Module Registry service based on the directory structure of https://bcr.bazel.build/ and the Git
* repository it is based on (https://github.com/bazelbuild/bazel-central-registry/). It uses [client] to send HTTP
* request to the registry at [baseUrl].

Check warning on line 38 in clients/bazel-module-registry/src/main/kotlin/RemoteBazelModuleRegistryService.kt

View workflow job for this annotation

GitHub Actions / qodana-scan

Unresolved reference in KDoc

Cannot resolve symbol 'baseUrl'
Dismissed Show dismissed Hide dismissed
*/
interface RemoteBazelModuleRegistryService : BazelModuleRegistryService {
class RemoteBazelModuleRegistryService(
private val client: RetrofitRegistryServiceClient,
baseUrl: String
) : BazelModuleRegistryService {
companion object {
/**
* Create a Bazel Module Registry client instance for communicating with a server running at the given [url],
Expand All @@ -58,24 +60,15 @@
.addConverterFactory(JSON.asConverterFactory(contentType))
.build()

return retrofit.create(RemoteBazelModuleRegistryService::class.java)
val retrofitClient = retrofit.create(RetrofitRegistryServiceClient::class.java)
return RemoteBazelModuleRegistryService(retrofitClient, baseUrl)
}
}

/**
* Retrieve the metadata for a module.
* E.g. https://bcr.bazel.build/modules/glog/metadata.json.
*/
@GET("modules/{name}/metadata.json")
override suspend fun getModuleMetadata(@Path("name") name: String): ModuleMetadata
override val urls: List<String> = listOf(baseUrl)

/**
* Retrieve information about the source code for a specific version of a module.
* E.g. https://bcr.bazel.build/modules/glog/0.5.0/source.json.
*/
@GET("modules/{name}/{version}/source.json")
override suspend fun getModuleSourceInfo(
@Path("name") name: String,
@Path("version") version: String
): ModuleSourceInfo
override suspend fun getModuleMetadata(name: String): ModuleMetadata = client.getModuleMetadata(name)

override suspend fun getModuleSourceInfo(name: String, version: String): ModuleSourceInfo =
client.getModuleSourceInfo(name, version)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Copyright (C) 2024 The ORT Project Authors (see <https://github.com/oss-review-toolkit/ort/blob/main/NOTICE>)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
* License-Filename: LICENSE
*/

package org.ossreviewtoolkit.clients.bazelmoduleregistry

import retrofit2.http.GET
import retrofit2.http.Path

/**
* A Retrofit client for remote bazel module registries.
*/
interface RetrofitRegistryServiceClient {
/**
* Retrieve the metadata for a module.
* E.g. https://bcr.bazel.build/modules/glog/metadata.json.
*/
@GET("modules/{name}/metadata.json")
suspend fun getModuleMetadata(@Path("name") name: String): ModuleMetadata

/**
* Retrieve information about the source code for a specific version of a module.
* E.g. https://bcr.bazel.build/modules/glog/0.5.0/source.json.
*/
@GET("modules/{name}/{version}/source.json")
suspend fun getModuleSourceInfo(@Path("name") name: String, @Path("version") version: String): ModuleSourceInfo
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
---
project:
id: "Bazel::plugins/package-managers/bazel/src/funTest/assets/projects/synthetic/bazel-no-lock-file/MODULE.bazel:"
definition_file_path: "<REPLACE_DEFINITION_FILE_PATH>"
declared_licenses: []
declared_licenses_processed: {}
vcs:
type: "Git"
url: "<REPLACE_URL_PROCESSED>"
revision: "<REPLACE_REVISION>"
path: "<REPLACE_PATH>"
vcs_processed:
type: "Git"
url: "<REPLACE_URL_PROCESSED>"
revision: "<REPLACE_REVISION>"
path: "<REPLACE_PATH>"
homepage_url: ""
scopes:
- name: "dev"
dependencies: []
- name: "main"
dependencies:
- id: "Bazel::glog:0.5.0"
linkage: "STATIC"
dependencies:
- id: "Bazel::gflags:2.2.2"
linkage: "STATIC"
packages:
- id: "Bazel::gflags:2.2.2"
purl: "pkg:generic/[email protected]"
declared_licenses: []
declared_licenses_processed: {}
description: ""
homepage_url: "https://gflags.github.io/gflags/"
binary_artifact:
url: ""
hash:
value: ""
algorithm: ""
source_artifact:
url: "https://github.com/gflags/gflags/archive/refs/tags/v2.2.2.tar.gz"
hash:
value: "34af2f15cf7367513b352bdcd2493ab14ce43692d2dcd9dfc499492966c64dcf"
algorithm: "SHA-256"
vcs:
type: "Git"
url: "https://github.com/gflags/gflags"
revision: ""
path: ""
vcs_processed:
type: "Git"
url: "https://github.com/gflags/gflags.git"
revision: ""
path: ""
- id: "Bazel::glog:0.5.0"
purl: "pkg:generic/[email protected]"
declared_licenses: []
declared_licenses_processed: {}
description: ""
homepage_url: "https://github.com/google/glog"
binary_artifact:
url: ""
hash:
value: ""
algorithm: ""
source_artifact:
url: "https://github.com/google/glog/archive/refs/tags/v0.5.0.tar.gz"
hash:
value: "eede71f28371bf39aa69b45de23b329d37214016e2055269b3b5e7cfd40b59f5"
algorithm: "SHA-256"
vcs:
type: "Git"
url: "https://github.com/google/glog"
revision: ""
path: ""
vcs_processed:
type: "Git"
url: "https://github.com/google/glog.git"
revision: ""
path: ""
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
common --lockfile_mode=off

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
7.0.1
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
bazel_dep(name = "glog", version = "0.5.0", repo_name = "com_github_google_glog")
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
cc_library(
name = "cookie-jar",
srcs = ["cookie-jar.cc"],
hdrs = ["cookie-jar.h"],
visibility = ["//main:__pkg__", "//test:__pkg__"],
)

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#include "lib/cookie-jar.h"

int grab_cookies() {
return 42;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#ifndef LIB_COOKIE_JAR_H_
#define LIB_COOKIE_JAR_H_

int grab_cookies();

#endif
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
cc_binary(
name = "main",
srcs = [
"main.cc",
],
deps = [
"//lib:cookie-jar",
"@com_github_google_glog//:glog",
],
)

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#include "lib/cookie-jar.h"
#include <glog/logging.h>

int main(int argc, char* argv[]) {
google::InitGoogleLogging(argv[0]);

int num_cookies = grab_cookies();
LOG(INFO) << "Found " << num_cookies << " cookies";
}

Original file line number Diff line number Diff line change
Expand Up @@ -93,4 +93,15 @@ class BazelFunTest : StringSpec({

result.toYaml() should matchExpectedResult(expectedResultFile, definitionFile)
}

"Dependencies are detected correctly even if no lock file is present and its generation is disabled" {
val definitionFile = getAssetFile("projects/synthetic/bazel-no-lock-file/MODULE.bazel")
val expectedResultFile = getAssetFile(
"projects/synthetic/bazel-expected-output-no-lock-file.yml"
)

val result = create("Bazel").resolveSingleProject(definitionFile)

result.toYaml() should matchExpectedResult(expectedResultFile, definitionFile)
}
})
4 changes: 2 additions & 2 deletions plugins/package-managers/bazel/src/main/kotlin/Bazel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,9 @@ class Bazel(
super.run(
args = args,
workingDir = workingDir,
// Disable the optional wrapper script under `tools/bazel`, to ensure the --version option works.
// Disable the optional wrapper script under `tools/bazel` only for the "--version" call.
Copy link
Member

Choose a reason for hiding this comment

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

can be relevant by defining

I don't understand this. Do you mean "can be relevant e.g. in case a specific '.bazelrc'
depending on the value of an environment variable is defined"?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried to explain the setup of our customer project and the phrasing is quite difficult...

They have in their project a wrapper script in tools/bazel. This script reads an environment variable and, depending on its value, change the .bazelrc of the project.
Therefore when you call bazel mod graph, this script "setups" your build environment depending on this environment variable.
However, bazel uses this script for all commands even if you call bazel version. In that case, the script fails because it does not support the --version parameter. While the correct fix would be to change the wrapper script to support this parameter, this is not possible to force all the projects to do so.
And, in all cases, ORT must make sure that the wrapper script is enabled when mod graph is called, as it may contain some build changing logic as described above.

environment = environment + mapOf(
"BAZELISK_SKIP_WRAPPER" to "true",
"BAZELISK_SKIP_WRAPPER" to "${args[0] == getVersionArguments()}",
"USE_BAZEL_FALLBACK_VERSION" to BAZEL_FALLBACK_VERSION
)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ internal class CompositeBazelModuleRegistryService(
}
}

override val urls = packagesPerRegistry.keys.flatMap { it.urls }

override suspend fun getModuleMetadata(name: String): ModuleMetadata {
val registry = packagesPerRegistry.entries.find { name in it.value }
?: throw IllegalArgumentException("No registry found for package '$name'.")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ package org.ossreviewtoolkit.plugins.packagemanagers.bazel

import java.io.File

import org.apache.logging.log4j.kotlin.logger
sschuberth marked this conversation as resolved.
Show resolved Hide resolved

import org.ossreviewtoolkit.clients.bazelmoduleregistry.BazelModuleRegistryService
import org.ossreviewtoolkit.clients.bazelmoduleregistry.DEFAULT_URL
import org.ossreviewtoolkit.clients.bazelmoduleregistry.LocalBazelModuleRegistryService
Expand Down Expand Up @@ -50,6 +52,8 @@ internal class MultiBazelModuleRegistryService(
// Add the default Bazel registry as a fallback.
val registryUrls = (urls + DEFAULT_URL).distinctBy { it.removeSuffix("/") }

logger.info { "Creating a multi Bazel module registry:" }
Copy link
Member

Choose a reason for hiding this comment

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

IMO this should be debug instead of info.

Copy link
Member Author

Choose a reason for hiding this comment

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

Since the logging occurs only one, I think there is no harm in letting it to info, especially considering how noisy debug is.


val registryServices = registryUrls.mapTo(mutableListOf()) { url ->
LocalBazelModuleRegistryService.create(url, projectDir)
?: RemoteBazelModuleRegistryService.create(url)
Expand All @@ -61,12 +65,14 @@ internal class MultiBazelModuleRegistryService(
/**
* Return an exception with a message that combines the messages of all [Throwable]s in this list.
*/
private fun List<Throwable>.combinedException(caption: String): Throwable =
private fun List<String>.combinedException(caption: String): Throwable =
IllegalArgumentException(
"$caption:\n${joinToString("\n") { it.message.orEmpty() }}"
"$caption:\n${joinToString("\n")}"
)
}

override val urls = registryServices.flatMap { it.urls }

override suspend fun getModuleMetadata(name: String): ModuleMetadata =
queryRegistryServices(
errorMessage = { "Failed to query metadata for package '$name'" },
Expand All @@ -88,20 +94,26 @@ internal class MultiBazelModuleRegistryService(
errorMessage: () -> String,
query: suspend (BazelModuleRegistryService) -> T
): T {
val failures = mutableListOf<Throwable>()
val failures = mutableListOf<String>()

tailrec suspend fun queryServices(itServices: Iterator<BazelModuleRegistryService>): T? =
if (!itServices.hasNext()) {
null
} else {
val triedResult = runCatching { query(itServices.next()) }
val nextRegistry = itServices.next()
val triedResult = runCatching { query(nextRegistry) }
val result = triedResult.getOrNull()

// The Elvis operator does not work here because of the tailrec modifier.
if (result != null) {
result
} else {
triedResult.exceptionOrNull()?.let(failures::add)
triedResult.exceptionOrNull()?.let {
failures += "${nextRegistry.urls.joinToString()}: ${
it.message.orEmpty().ifEmpty { "<no message>" }
}"
}

queryServices(itServices)
}
}
Expand Down
Loading
Loading