Skip to content
This repository has been archived by the owner on Aug 5, 2024. It is now read-only.

[feature] Go support #507

Draft
wants to merge 41 commits into
base: master
Choose a base branch
from
Draft

Conversation

zortenburger
Copy link

BAZEL-749

MODULE.bazel Outdated
@@ -16,6 +16,7 @@ bazel_dep(name = "rules_proto", version = "5.3.0-21.7")
bazel_dep(name = "rules_cc", version = "0.0.9")
bazel_dep(name = "platforms", version = "0.0.8")
bazel_dep(name = "rules_python", version = "0.27.1")
bazel_dep(name = "rules_go", version = "0.43.0")
Copy link
Member

Choose a reason for hiding this comment

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

i think you dont need it

# Conflicts:
#	MODULE.bazel
# Conflicts:
#	protocol/src/main/kotlin/org/jetbrains/bsp/utils/Extractors.kt
#	server/src/main/kotlin/org/jetbrains/bsp/bazel/server/bsp/managers/BazelBspLanguageExtensionsGenerator.kt
#	server/src/main/kotlin/org/jetbrains/bsp/bazel/server/common/ServerContainer.kt
#	server/src/main/kotlin/org/jetbrains/bsp/bazel/server/sync/languages/LanguagePluginsService.kt
#	server/src/main/kotlin/org/jetbrains/bsp/bazel/server/sync/model/Language.kt
#	server/src/main/kotlin/org/jetbrains/bsp/bazel/server/sync/proto/bsp_target_info.proto
#	server/src/test/kotlin/org/jetbrains/bsp/bazel/server/sync/languages/LanguagePluginServiceTest.kt
Copy link
Member

@abrams27 abrams27 left a comment

Choose a reason for hiding this comment

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

some small things, so far looks good

load("//aspects:utils/utils.bzl", "create_proto", "create_struct", "file_location")
load("@io_bazel_rules_go//go:def.bzl", "go_context")

def extract_sdk(ctx):
Copy link
Member

Choose a reason for hiding this comment

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

let's put an _ to mark that's "private" and put the definition below its usage - so just put it under the extract_go_info

return None
return file_location(go.sdk.go)

def extract_go_info(target, ctx, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

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

you also need to check is it a go target (we usually check is there a language provider in the target, if not then we return None, None)

return file_location(go.sdk.go)

def extract_go_info(target, ctx, **kwargs):
importpath = getattr(ctx.rule.attr, "importpath", [])
Copy link
Member

Choose a reason for hiding this comment

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

importpath is a string, so the default fallback should be an empty string (or none)


data class GoBuildTarget(
val sdkHomePath: URI?,
val importPath: String?,
Copy link
Member

Choose a reason for hiding this comment

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

question is: can target have no import path?

data class GoModule (
val sdkHomePath: URI?,
val importPath: String?,
): LanguageData
Copy link
Member

Choose a reason for hiding this comment

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

new line

val goSdkDir = goBinaryPath.parent.parent
goSdkDir.toUri()
}
}
Copy link
Member

Choose a reason for hiding this comment

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

new line

@abrams27
Copy link
Member

abrams27 commented Mar 14, 2024

plus you can also rebase it and try to add e2e tests

# Conflicts:
#	MODULE.bazel.lock
#	server/src/main/kotlin/org/jetbrains/bsp/bazel/server/bsp/managers/BazelBspLanguageExtensionsGenerator.kt
#	server/src/main/kotlin/org/jetbrains/bsp/bazel/server/sync/BazelProjectMapper.kt
#	server/src/main/kotlin/org/jetbrains/bsp/bazel/server/sync/TargetInfoReader.kt
#	server/src/main/kotlin/org/jetbrains/bsp/bazel/server/sync/model/Language.kt
#	server/src/main/kotlin/org/jetbrains/bsp/bazel/server/sync/proto/bsp_target_info.proto
Copy link
Member

@abrams27 abrams27 left a comment

Choose a reason for hiding this comment

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

small thing

maclick and others added 10 commits April 12, 2024 01:23
…k/go-support

# Conflicts:
#	server/src/main/kotlin/org/jetbrains/bsp/bazel/server/BazelBspServer.kt
#	server/src/test/kotlin/org/jetbrains/bsp/bazel/server/sync/languages/LanguagePluginServiceTest.kt
# Conflicts:
#	MODULE.bazel.lock
#	server/src/test/kotlin/org/jetbrains/bsp/bazel/server/bsp/managers/BazelBspLanguageExtensionsGeneratorTest.kt
edokimok and others added 15 commits April 24, 2024 15:38
# Conflicts:
#	server/src/main/kotlin/org/jetbrains/bsp/bazel/server/sync/BazelProjectMapper.kt
#	server/src/main/kotlin/org/jetbrains/bsp/bazel/server/sync/BspProjectMapper.kt
…support

# Conflicts:
#	.bazelrc
#	server/src/main/kotlin/org/jetbrains/bsp/bazel/server/sync/BazelProjectMapper.kt
# Conflicts:
#	.bazelrc
#	server/src/main/kotlin/org/jetbrains/bsp/bazel/server/sync/BazelProjectMapper.kt
#	server/src/main/kotlin/org/jetbrains/bsp/bazel/server/sync/TargetInfoReader.kt
# Conflicts:
#	.bazelrc
#	aspects/rules/go/go_info.bzl.template
#	server/src/main/kotlin/org/jetbrains/bsp/bazel/server/sync/BazelProjectMapper.kt
Copy link
Member

@abrams27 abrams27 left a comment

Choose a reason for hiding this comment

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

looks reallny nice, some minor comments

Comment on lines +11 to +12
val goImportPath: String? = "",
val goRoot: URI? = URI(""),
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 null since it's nullable

Copy link

Choose a reason for hiding this comment

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

makes sense

Comment on lines +12 to +13
val goImportPath: String? = "",
val goRoot: URI? = URI(""),
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 null as ewll

importpath = go_archive.data.importpath
sdk_home_path = _extract_sdk(ctx)

go_target_info = create_struct(
Copy link
Member

Choose a reason for hiding this comment

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

pls dont forget about embeds

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants