Conversation
Summary of ChangesHello @JassJam, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces initial support for building C# projects within the xmake build system. It establishes the foundational language definition and build rules for C#, enabling xmake to compile and publish C# applications and libraries using the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
The pull request introduces C# support to xmake, including basic build, clean, and install functionalities for various project types like console applications, libraries, and web projects. The implementation correctly handles .csproj files and integrates with the dotnet CLI. The changes are well-structured and provide a good foundation for C# support.
|
I've tried it, and the verbose logs should only show detailed build commands and warnings. Other irrelevant logs shouldn't be output. PS D:\projects\personal\xmake\tests\projects\csharp\console> xmake -rv
checking for platform ... windows (x64)
checking for cl.exe ... C:\Program Files\Microsoft Visual Studio\18\Insiders\VC\Tools\MSVC\14.50.35503\bin\HostX64\x64\cl.exe
checking for Microsoft C/C++ Compiler (x64) ... ok
checking for Microsoft Visual Studio (x64) version ... 2026
checking for nim ... no
[ 31%]: building.csharp.release test
dotnet publish D:\projects\personal\xmake\tests\projects\csharp\console\src\test.csproj --nologo --configuration Release --verbosity minimal --output D:\projects\personal\xmake\tests\projects\csharp\console\build\windows\x64\release\test --runtime win-x64
欢迎使用 .NET 10.0!
---------------------
SDK 版本: 10.0.100-rc.1.25451.107
遥测
---------
.NET 工具会收集用法数据，帮助我们改善你的体验。它由 Microsoft 收集并与社区共享。你可通过使用喜欢的 shell 将 DOTNET_CLI_TELEMETRY_OPTOUT 环境变量设置为 "1" 或 "true" 来选择退出遥测。
阅读有关 .NET CLI 工具遥测的更多信息: https://aka.ms/dotnet-cli-telemetry
----------------
已安装 ASP.NET Core HTTPS 开发证书。
若要信任该证书，请运行 "dotnet dev-certs https --trust"
了解 HTTPS: https://aka.ms/dotnet-https
----------------
编写第一个应用: https://aka.ms/dotnet-hello-world
了解新增功能: https://aka.ms/dotnet-whats-new
浏览文档: https://aka.ms/dotnet-docs
报告问题并在 GitHub 上查找来源: https://github.com/dotnet/core
使用 "dotnet --help" 查看可用命令或访问: https://aka.ms/dotnet-cli
--------------------------------------------------------------------------------------
还原完成(0.8)
info NETSDK1057: 你正在使用 .NET 的预览版。请参阅 https://aka.ms/dotnet-support-policy
test 已成功 (4.3 秒) → D:\projects\personal\xmake\tests\projects\csharp\console\build\windows\x64\release\test\
在 6.0 秒内生成 已成功
checking for cl.exe ... C:\Program Files\Microsoft Visual Studio\18\Insiders\VC\Tools\MSVC\14.50.35503\bin\HostX64\x64\cl.exe
checking for the c compiler (cc) ... cl.exe
[100%]: build ok, spent 8.031sExpectedPS D:\projects\personal\xmake\tests\projects\csharp\console> xmake -rv
checking for platform ... windows (x64)
checking for cl.exe ... C:\Program Files\Microsoft Visual Studio\18\Insiders\VC\Tools\MSVC\14.50.35503\bin\HostX64\x64\cl.exe
checking for Microsoft C/C++ Compiler (x64) ... ok
checking for Microsoft Visual Studio (x64) version ... 2026
checking for nim ... no
[ 31%]: building.csharp.release test
dotnet publish D:\projects\personal\xmake\tests\projects\csharp\console\src\test.csproj --nologo --configuration Release --verbosity minimal --output D:\projects\personal\xmake\tests\projects\csharp\console\build\windows\x64\release\test --runtime win-x64
checking for cl.exe ... C:\Program Files\Microsoft Visual Studio\18\Insiders\VC\Tools\MSVC\14.50.35503\bin\HostX64\x64\cl.exe
checking for the c compiler (cc) ... cl.exe
[100%]: build ok, spent 8.031s |
|
Incremental compilation doesn't seem to be working. PS D:\projects\personal\xmake\tests\projects\csharp\exe_with_library> xmake
[ 15%]: building.csharp.release mylib
[ 63%]: building.csharp.release app
[100%]: build ok, spent 4.515s
PS D:\projects\personal\xmake\tests\projects\csharp\exe_with_library> xmake
[ 15%]: building.csharp.release mylib
[ 63%]: building.csharp.release app |
| <ImplicitUsings>enable</ImplicitUsings> | ||
| <Nullable>enable</Nullable> | ||
| </PropertyGroup> | ||
| </Project> |
There was a problem hiding this comment.
This requires users to maintain both xmake.lua and csproj. Build support for other languages only requires users to maintain xmake.lua. I believe this can be automatically generated in the rule, eliminating the need for users to maintain it separately.
My initial idea was to completely eliminate the dependency on csproj, directly calling the csc command to compile them. Just like in C++, completely getting rid of msbuild and sln.
There was a problem hiding this comment.
I see your point, however in my opnion this will be too large and volatile to keep up with .NET sdk changes, there is also the fact that dotnet command that generates templated projects, add packages, restore... etc, touches the csproj file directly.
I don't know if it'll be an easy feat / worth the effort to handle all what dotnet already does.
There was a problem hiding this comment.
I believe it's worthwhile. xmake's initial purpose was to simplify project configuration as much as possible and reduce the maintenance burden on users.
Furthermore, this doesn't conflict with the current implementation; if the user explicitly specifies the add_files("*.csproj") configuration, xmake can still prioritize using it.
What I mean is, it at least provides a default solution. In most basic project configurations, xmake can support builds well even if the user doesn't provide a csproj file.
And for some complex requirements, users can decide whether to provide an external csproj file.
Updated, can you test it again? |
There seems to be some additional output. PS D:\projects\personal\xmake\tests\projects\csharp\web_project> xmake -rv
[ 31%]: building.csharp.release webapp
dotnet publish D:\projects\personal\xmake\tests\projects\csharp\web_project\src\webapp.csproj --nologo --configuration Release --verbosity minimal --output D:\projects\personal\xmake\tests\projects\csharp\web_project\build\windows\x64\release\webapp --runtime win-x64
还原完成(0.7)
info NETSDK1057: 你正在使用 .NET 的预览版。请参阅 https://aka.ms/dotnet-support-policy
webapp 已成功 (6.0 秒) → D:\projects\personal\xmake\tests\projects\csharp\web_project\build\windows\x64\release\webapp\
在 7.8 秒内生成 已成功
[100%]: build ok, spent 9.719s
PS D:\projects\personal\xmake\tests\projects\csharp\web_project> |
xmake/core/project/policy.lua
Outdated
| -- Set the build progress output style, e.g. scroll (default), singlerow, multirow | ||
| ["build.progress_style"] = {description = "Set the build progress output style.", type = "string", values = {"scroll", "singlerow", "multirow"}}, | ||
| -- Set dotnet verbosity for csharp rule, e.g. quiet|minimal|normal|detailed|diagnostic | ||
| ["build.csharp.dotnet_verbosity"] = {description = "Set dotnet verbosity for csharp rule.", type = "string", values = {"quiet", "minimal", "normal", "detailed", "diagnostic"}}, |
There was a problem hiding this comment.
Please do not add any additional strategies to control log output. xmake has its own log output levels, verbose, and diagnosis. Additional log levels will only confuse users.
We can use diagnosis to control the extra logs output of dotnet. xmake f -D
import("core.base.option")
if option.get("diagnosis") then
endor use dprint
There was a problem hiding this comment.
hopefully this fixes the xmake -rv, i wasn't getting similar result.
PS C:\Users\Jam\Desktop\projects\xmake\tests\projects\csharp\web_project> xmake -rv
checking for platform ... windows (x64)
checking for cl.exe ... C:\Program Files\Microsoft Visual Studio\18\Enterprise\VC\Tools\MSVC\14.50.35717\bin\HostX64\x64\cl.exe
checking for Microsoft C/C++ Compiler (x64) ... ok
checking for Microsoft Visual Studio (x64) version ... 2026
checking for nim ... no
[ 31%]: building.csharp.release webapp
dotnet publish C:\Users\Jam\Desktop\projects\xmake\tests\projects\csharp\web_project\src\webapp.csproj --nologo --configuration Release --verbosity quiet --output C:\Users\Jam\Desktop\projects\xmake\tests\projects\csharp\web_project\build\windows\x64\release\webapp --runtime win-x64
checking for cl.exe ... C:\Program Files\Microsoft Visual Studio\18\Enterprise\VC\Tools\MSVC\14.50.35717\bin\HostX64\x64\cl.exe
checking for the c compiler (cc) ... cl.exe
[100%]: build ok, spent 1.25s
PS C:\Users\Jam\Desktop\projects\xmake\tests\projects\csharp\web_project>
|
Additionally, I think it's entirely possible to remove the csproj files and have them automatically generated within the rules. |
I agree that it's possible, but I don't think that'll be a good idea. This will add alot of overhead in generating the csproj and maintaining/keeping up with the dotnet ecosystem, are we sure we want to go with it, or should we make it a seperate PR/TODO for later? |
This is absolutely necessary. Otherwise, if support for other language projects like Swift requires users to provide additional .xcodeproj files, then using xmake becomes completely pointless. It would be better to let users directly use xcode or msbuild to build them. Similarly, if building a CSharp project requires additional maintenance of the csproj file, then using xmake to build it is pointless. Users can simply use Visual Studio to build it directly.
Since we already support the vsproj generator, generating project files inherently relies heavily on the Visual Studio and .NET ecosystem. This is unavoidable.
Supporting it in the current pull request is the most basic requirement. Then, the test examples provide both examples with and without the |
|
The new commits added csproj lookup if it was provided with fallback to csproj generation if it wasnt provided. Added various commonly used csproj properties nuget's The nuget's refactor was done because i was getting the following error: |
|
Thanks, I need some time to review. |
|
When csproj exists, the source code directory will generate an out/bin/obj directory instead of the build directory. It should use the paths specified by target:objectdir, target:targetdir, and target:targetfile.
|
|
|
||
| target("test") | ||
| set_kind("binary") | ||
| add_rules("csharp") |
There was a problem hiding this comment.
We have already defined the .cs language, and this rule will be applied automatically; we don't need to set it.
|
|
||
| var runtimeFile = Path.Combine(AppContext.BaseDirectory, "runtime.json"); | ||
| if (!File.Exists(runtimeFile)) | ||
| { |
There was a problem hiding this comment.
Ensure your coding style is consistent with xmake/core/c code.
if () {
}| namespace MyLib; | ||
|
|
||
| public static class Greeter | ||
| { |
|
|
||
| target("app") | ||
| set_kind("binary") | ||
| add_rules("csharp") |
|
|
||
| target("app") | ||
| set_kind("binary") | ||
| add_rules("csharp") |
|
|
||
| target("test") | ||
| set_kind("binary") | ||
| add_rules("csharp") |
| -- @file load.lua | ||
| -- | ||
|
|
||
| import("modules.csharp_common", {rootdir = os.scriptdir(), alias = "csharp_common"}) |
| -- @file installcmd.lua | ||
| -- | ||
|
|
||
| import("modules.csharp_common", {rootdir = os.scriptdir(), alias = "csharp_common"}) |
| batchcmds:mkdir(install_abs) | ||
| end | ||
| batchcmds:vrunv(dotnet, argv, csharp_common.get_dotnet_runopt(csprojfile)) | ||
| end |
There was a problem hiding this comment.
installcmd is for Xpack. Have you tested Xpack? Can it successfully generate a CSharp NSIS installer?
| -- @file install.lua | ||
| -- | ||
|
|
||
| import("modules.csharp_common", {rootdir = os.scriptdir(), alias = "csharp_common"}) |
| csharp_common.append_target_flags(target, argv) | ||
|
|
||
| local runopt = csharp_common.get_dotnet_runopt(csprojfile) | ||
| if os.vrunv then |
There was a problem hiding this comment.
why? They always exist; we only need to call one. os.vrunv
|
The current build implementation is inconsistent with the current architecture. Please refer to the implementation of Rust rules. use compiler:build and compiler:buildcmd xmake/xmake/rules/rust/build/target.lua Lines 60 to 67 in 71e10b7 then add https://github.com/xmake-io/xmake/blob/dev/xmake/modules/core/tools/rustc.lua xmake/xmake/modules/core/tools/rustc.lua Line 32 in 71e10b7 |
| if rid and target:is_binary() then | ||
| table.join2(argv, {"--runtime", rid}) | ||
| end | ||
| csharp_common.append_target_flags(target, argv) |
There was a problem hiding this comment.
We should move build logic to core/tools/donet.lua, Implementations such as nf_optimize will be automatically mapped to build flags when call compiler:build
| if targetdirabs then | ||
| batchcmds:mkdir(targetdirabs) | ||
| end | ||
| batchcmds:vrunv(dotnet, argv, csharp_common.get_dotnet_runopt(csprojfile)) |
There was a problem hiding this comment.
we can use batchcmds:compile in buildcmd instead of compiler:build.
| end | ||
| if not major then | ||
| local version = try { function () | ||
| return os.iorunv(dotnet, {"--version"}) |
There was a problem hiding this comment.
we should implment it in find_dotnet, then use find_tool("dotnet", {version = true}) to get version
| end | ||
|
|
||
| function _get_default_target_framework(context) | ||
| local dotnet = _first(context.target:get("toolset.cs")) or "dotnet" |
|
Additionally, we should define a new toolchain toolchains/dotnet.lua and its corresponding has_flags.lua implementation. You can refer to https://github.com/xmake-io/xmake/tree/dev/tests/apis/custom_toolchain/xmake |
|
Perhaps I haven't explained it clearly enough, but these things are essential for supporting a new language.
|
|
wait for this patch #7398 |
|
Works fine on my windows and linux machine, good work @waruqi !! |
Based on #7338.
This is part 1, an implementation using basic xmake cli options with tests.