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

.NET C# no way to enable different .NET sdk's on .netcore projects! #2289

Open
wants to merge 54 commits into
base: master
Choose a base branch
from

Conversation

lolrobbe2
Copy link
Contributor

@lolrobbe2 lolrobbe2 commented Oct 7, 2024

What does this PR do?

Add support for setting .NET sdk using dotnetsdk option. (only on new format projects otherwise just use the defautl)

Web
Razor
Worker
Blazor (BlazorWebAssembly)
WindowsDesktop
default (Microsoft.NET.Sdk)

How does this PR change Premake's behavior?

adds a new option called dotnetsdk

Anything else we should know?

Add any other context about your changes here.

Did you check all the boxes?

  • Focus on a single fix or feature; remove any unrelated formatting or code changes
  • Add unit tests showing fix or feature works; all tests pass
  • Mention any related issues (put closes #XXXX in comment to auto-close issue when PR is merged)
  • Follow our coding conventions
  • Minimize the number of commits
  • Align documentation to your changes

You can now support Premake on our OpenCollective. Your contributions help us spend more time responding to requests like these!

for some reason it took the commits from my previous pr (add C# documentation file to) i didn't change these files tho!

closes #2288

robbe beernaert and others added 30 commits June 29, 2023 17:49
(functional result stays the same)
modules/vstudio/vs2005_dotnetbase.lua Outdated Show resolved Hide resolved
modules/vstudio/vs2005_dotnetbase.lua Outdated Show resolved Hide resolved
website/docs/dotnetsdk.md Outdated Show resolved Hide resolved
src/_premake_init.lua Outdated Show resolved Hide resolved
@lolrobbe2 lolrobbe2 requested a review from Jarod42 October 9, 2024 13:21
<Project Sdk="Microsoft.NET.Sdk.WindowsDesktop">
]]
end
function suite.testMSTest()
Copy link
Contributor

Choose a reason for hiding this comment

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

global.json part is not tested

Copy link
Contributor Author

Choose a reason for hiding this comment

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

how would i go about that?

Copy link
Contributor

Choose a reason for hiding this comment

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

Issue is probably that global.json doesn't follow other file generation and so dotnetbase.netcore.dotnetsdk would be misplaced...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

would you happen to know where i can find another example of this file generating?

Copy link
Contributor

Choose a reason for hiding this comment

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

I though about premake.generate, something like:

local content = p.capture(function() generate_global_json_content(prj) end)
if content ~= nil and #content > 0 then
	p.generate(prj, "global.json", function() p.outln(content) end)
end

That way, generate_global_json_content can be tested.
User is notified of the creation of the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i got premake.generate to work except the test is not working probably somthing stupid that i am missing!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image
i got this far but it seems the content is not getting captured properly

@lolrobbe2 lolrobbe2 requested a review from Jarod42 October 9, 2024 14:54
Beernaert Robbe added 2 commits October 9, 2024 18:11
…in test case but is written in normal ussage
…ause the file is created fine when using in non test environment!

also tried changing dir to _TESTS_DIR to no avail.
@lolrobbe2
Copy link
Contributor Author

Could it be that tests are blocked from creating files? wich would explain why the file is not found!

@Jarod42
Copy link
Contributor

Jarod42 commented Oct 10, 2024

Could it be that tests are blocked from creating files? what would explain why the file is not found!

There is indeed test_runner.lua which installs custom hooks to avoid file creation.
Test should not use file, but check what they output to them.

Comment on lines 838 to 846
-- do not remove the file generation for global.json otherwise MSTest will not work!!!
local globalpath = path.join(cfg.workspace.location, "global.json")
if cfg.dotnetsdk == "MSTest" and not os.isfile(globalpath) then
io.writefile(globalpath, '{"msbuild-sdks": {"MSTest.Sdk": "3.6.1"}}')
local content = p.capture(function() generate_global_json_content(prj) end)
if content ~= nil and #content > 0 then
p.generate(cfg.workspace, path.join(cfg.workspace.location,"global.json"), function() p.outln(content) end)
end
end
end
Copy link
Contributor

Choose a reason for hiding this comment

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

I meant:

Suggested change
-- do not remove the file generation for global.json otherwise MSTest will not work!!!
local globalpath = path.join(cfg.workspace.location, "global.json")
if cfg.dotnetsdk == "MSTest" and not os.isfile(globalpath) then
io.writefile(globalpath, '{"msbuild-sdks": {"MSTest.Sdk": "3.6.1"}}')
local content = p.capture(function() generate_global_json_content(prj) end)
if content ~= nil and #content > 0 then
p.generate(cfg.workspace, path.join(cfg.workspace.location,"global.json"), function() p.outln(content) end)
end
end
end
function dotnetbase.output_global_json(prj) -- unsure how you handle project with different config
if cfg.dotnetsdk == "MSTest" then
_p('{"msbuild-sdks": {"MSTest.Sdk": "3.6.1"}}')
end
end
function dotnetbase.generate_global_json(prj)
local content = p.capture(function() dotnetbase.output_global_json(prj) end)
if content ~= nil and #content > 0 then
p.generate(prj, path.join(prj.workspace.location,"global.json"), function() p.outln(content) end)
end
end

In vs2005.generateProject(prj)
Add dotnetbase.generate_global_json(prj) in iscsharp and in isfsharp branch.

In test, just check output of dotnetbase.output_global_json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ill give it try!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

getting somewhere but now other test is broken

Copy link
Contributor Author

@lolrobbe2 lolrobbe2 Oct 10, 2024

Choose a reason for hiding this comment

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

fixed it it is now working!! btw thanks for the help!!!

@lolrobbe2
Copy link
Contributor Author

the error in mingw is because it cannot connect to nuget services!

@lolrobbe2
Copy link
Contributor Author

lolrobbe2 commented Oct 10, 2024

question: when is beta 3 planned to be released because i would like to have a look a the platforms bug and see if i can fix it potentially?

function dotnetbase.allowUnsafeBlocks(cfg)
if cfg.clr == "Unsafe" then
_p(2,'<AllowUnsafeBlocks>true</AllowUnsafeBlocks>')
end
end

function dotnetbase.output_global_json(prj) -- unsure how you handle project with different config
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment was for you ;-)
Does it make sense that user have something like

filter "configuration:Release"
    dotnetsdk "Web"
filter "configuration:Test"
    dotnetsdk "MSTest"

How do you handle it?
As it seems global.json would be ignored with other dotnetsdk, checking that at least one cfg has this flag might be the solution

Copy link
Contributor Author

@lolrobbe2 lolrobbe2 Oct 12, 2024

Choose a reason for hiding this comment

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

no not really as they are seperate frameworks i see absolutly no reason ass it imports different namespaces etc and would break code!
global.json is also only needed to define the version of the MSTest sdk other sdk's do this automatically

function dotnetbase.generate_global_json(prj)
local content = p.capture(function() dotnetbase.output_global_json(prj) end)
if content ~= nil and #content > 0 then
p.generate(prj, path.join(prj.workspace.location,"global.json"), function() p.outln(content) end)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
p.generate(prj, path.join(prj.workspace.location,"global.json"), function() p.outln(content) end)
p.generate(prj, path.join(prj.workspace.location, "global.json"), function() p.outln(content) end)

BTW, I removed your check to not overwrite existing file for simplicity in precedent suggestion.
Unsure of your expected behavior with existing file.
If you keep existing code, doc should be updated as existing file would be overwritten.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

preferebly you would just append it to the global.json file or update it but have no idea on how to acomplish that with existing json api's

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gonna have a look at json.decode and encode to make it so it just updates the version or generates it when missing!

Comment on lines 849 to 859
globaljson = json.decode(io.readfile(path.join(prj.workspace.location, "global.json")))
if globaljson == nil then
globaljson = {}
globaljson["msbuild-sdks"] = {}
globaljson["msbuild-sdks"]["MSTest.Sdk"] = "3.6.1"
elseif globaljson["msbuild-sdks"] ~= nil then
globaljson["msbuild-sdks"]["MSTest.Sdk"] = "3.6.1"
else
globaljson["msbuild-sdks"] = {}
globaljson["msbuild-sdks"]["MSTest.Sdk"] = "3.6.1"
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
globaljson = json.decode(io.readfile(path.join(prj.workspace.location, "global.json")))
if globaljson == nil then
globaljson = {}
globaljson["msbuild-sdks"] = {}
globaljson["msbuild-sdks"]["MSTest.Sdk"] = "3.6.1"
elseif globaljson["msbuild-sdks"] ~= nil then
globaljson["msbuild-sdks"]["MSTest.Sdk"] = "3.6.1"
else
globaljson["msbuild-sdks"] = {}
globaljson["msbuild-sdks"]["MSTest.Sdk"] = "3.6.1"
end
globaljson = json.decode(io.readfile(path.join(prj.workspace.location, "global.json")))
globaljson = globaljson or {}
globaljson["msbuild-sdks"] = globaljson["msbuild-sdks"] or {}
globaljson["msbuild-sdks"]["MSTest.Sdk"] = "3.6.1"

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

Successfully merging this pull request may close these issues.

3 participants