premake-core icon indicating copy to clipboard operation
premake-core copied to clipboard

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

Open lolrobbe2 opened this issue 1 year ago • 4 comments

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?

  • [x] Focus on a single fix or feature; remove any unrelated formatting or code changes
  • [x] Add unit tests showing fix or feature works; all tests pass
  • [x] Mention any related issues (put closes #XXXX in comment to auto-close issue when PR is merged)
  • [x] Follow our coding conventions
  • [ ] Minimize the number of commits
  • [x] 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

lolrobbe2 avatar Oct 07 '24 17:10 lolrobbe2

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

lolrobbe2 avatar Oct 09 '24 20:10 lolrobbe2

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.

Jarod42 avatar Oct 10 '24 12:10 Jarod42

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

lolrobbe2 avatar Oct 10 '24 19:10 lolrobbe2

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?

lolrobbe2 avatar Oct 10 '24 20:10 lolrobbe2

fair enough but how would i go about that? being able to specify the version but only allow it to be specified with the sdks? but there is also customs sdks. wich would then not work and it would be prefereble that custom sdks work to.

lolrobbe2 avatar Nov 17 '24 13:11 lolrobbe2

fair enough but how would i go about that? being able to specify the version but only allow it to be specified with the sdks? but there is also customs sdks. wich would then not work and it would be prefereble that custom sdks work to.

I might not be understanding what you're ask, but there's a few ways to handle the versioning, each has it's own drawbacks.

Firstly, you could just drop the entire allowed part of the API and allow users to specify anything. You could also define a format of something like <sdk>/<version> which would allow you to still perform the lookup using <sdk> to save users having to type out Microsoft.NET. for all of the APIs. This approach would give the most flexibility, but it also means any spelling mistakes from a user would go unnoticed by Premake and they would be relying on VS to provide a meaningful error.

Secondly, you could create a second API to handle dotnetsdkversion and just ignore it if a user doesn't specify dotnetsdk. We ignore cppdialect when a user generates a C# project, so there's no real difference. This approach probably works the best, but it does mean the two values are specified separately and that can sometimes cause problems. This isn't an issue for language "C++" and cppdialect "C++14", so shouldn't be an issue here either.

Thirdly, you could do something similar to toolset which has a restricted set of values but also allows for msc-<version> where <version> isn't restricted. This one is probably the trickier one to implement while still allowing users to add new values that support versions. This can be achieved with something like this:

allowed = {
  "Default",
  "Web",
  "Razor",
  "Worker",
  "Blazor",
  "WindowsDesktop",
  "MSTest",
  function (value, kind)
    -- value is expected to be in the format <sdk>/<version>
    local parts = value:explode("/", true, 1)

    -- Only perform the check when a version is provided to avoid infinite recursing.
    if parts[2] == nil then
      return nil
    end

    -- Check that the specified sdk is in the allowed list
    if api.checkValue(p.field.get("dotnetsdk", parts[1], kind) then
      return value
    end

    -- The specified sdk isn't valid
    return nil
  end,
},

Let me know if that doesn't answer your questions, or if any of it didn't make sense.

samsinsane avatar Nov 18 '24 03:11 samsinsane

i went with the toolset way of doing things, there is also support for providing custom sdks and the previous way of just specifying the sdk still works! thanks for the help and advice @Jarod42 & @samsinsane!!!

lolrobbe2 avatar Nov 21 '24 17:11 lolrobbe2

now that dotnedSdk can specify WPF projects the WPF flag can be deprecated if i am not mistaken, just don't know how to go about that!

lolrobbe2 avatar Nov 21 '24 17:11 lolrobbe2

the provide snippet wont work because of how it is setup currently ill try and change it so it works

	function api.addAllowed(fieldName, value)
		local field = p.field.get(fieldName)
		if not field then
			error("No such field: " .. fieldName, 2)
		end

		if type(value) == "table" then
			for i, item in ipairs(value) do
				api.addAllowed(fieldName, item)
			end
		else
			field.allowed = field.allowed or {}

			-- If we are trying to add where the current value is a function,
			-- put the function in a table
			if type(field.allowed) == "function" then
				field.allowed = { field.allowed }
			end

			if field.allowed[value:lower()] == nil then
				table.insert(field.allowed, value)
				field.allowed[value:lower()] = value
			end
		end
	end

full code snippet:

	function api.register(field)
		-- verify the name
		local name = field.name
		if not name then
			error("missing name", 2)
		end

		if rawget(_G, name) then
			error("name '" .. name .. "' in use", 2)
		end

		-- add this new field to my master list
		field, err = p.field.new(field)
		if not field then
			error(err)
		end


		-- Flag fields which contain filesystem paths. The context object will
		-- use this information when expanding tokens, to ensure that the paths
		-- are still well-formed after replacements.

		field.paths = p.field.property(field, "paths")

		-- Add preprocessed, lowercase keys to the allowed and aliased value
		-- lists to speed up value checking later on.

		if type(field.allowed) == "table" then
			for i, item in ipairs(field.allowed) do
				field.allowed[item:lower()] = item
			end
		end

		if type(field.aliases) == "table" then
			local keys = table.keys(field.aliases)
			for i, key in ipairs(keys) do
				field.aliases[key:lower()] = field.aliases[key]
			end
		end

		-- create a setter function for it
		_G[name] = function(value)
			return api.storeField(field, value)
		end

		if p.field.removes(field) then
			_G["remove" .. name] = function(value)
				return api.remove(field, value)
			end
		end

		return field
	end

offending snippet from api.register.

	if type(field.allowed) == "table" then
		for i, item in ipairs(field.allowed) do
			field.allowed[item:lower()] = item
		end
	end

lolrobbe2 avatar Nov 22 '24 10:11 lolrobbe2

Tests failed:

[ FAILED ] vstudio_cs2005_dotnetsdk.testBlazorVersion /home/runner/work/premake-core/premake-core/src/base/api.lua:646: attempt to index a nil value (local 'value')

Jarod42 avatar Nov 25 '24 10:11 Jarod42

that is the issue for some reason the value is nill

image

lolrobbe2 avatar Nov 25 '24 11:11 lolrobbe2

Can you resolve the merge conflict?

nickclark2016 avatar Nov 26 '24 19:11 nickclark2016

update: i think the issue is with the fact that it does not do dictionary values: @samsinsane, @tritao

so the solution would probaly be to have a portion where it handles the dictionary values because if i looked it up right ipairs only returns the indexed pairs

api.register snippet in _preload

	p.api.register {
		name = "dotnetsdk",
		scope = "project",
		kind = "string",
		allowed = {
			"Default",
			"Web",
			"Razor",
			"Worker",
			"Blazor",
			"WindowsDesktop",
			"MSTest",
			dynamicValidator = function (value)
				-- value is expected to be in the format <sdk>/<version>
				local parts = value:explode("/", true, 1)
				local sdk = parts[1] or value


				if parts and #parts == 2 then
					if p.api.checkValue(p.field.get("dotnetsdk"), parts[1], "string") then
						return value
					end
				end

				return nil
			end
		}
	}

api.register snippet:

for i, item in ipairs(field.allowed) do
    if type(item) == "string" then
         field.allowed[item:lower()] = item
    end	
end

api.addAllowed

function api.addAllowed(fieldName, value)
		local field = p.field.get(fieldName)
		if not field then
			error("No such field: " .. fieldName, 2)
		end

		if type(value) == "table" then
			for i, item in ipairs(value) do
				api.addAllowed(fieldName, item)
			end
		else
			field.allowed = field.allowed or {}

			-- If we are trying to add where the current value is a function,
			-- put the function in a table
			if type(field.allowed) == "function" then
				field.allowed = { field.allowed }
			end

			if field.allowed[value:lower()] == nil then
				table.insert(field.allowed, value)
				field.allowed[value:lower()] = value
			end
		end
	end

lolrobbe2 avatar Jan 03 '25 14:01 lolrobbe2

all outstanding changes should be fixed.

lolrobbe2 avatar Jan 06 '25 11:01 lolrobbe2

There are still outstanding changes requested from previous reviews.

@samsinsane all outstanding changes should have been take care of. Can you pls look over everthing pls?

lolrobbe2 avatar Jan 08 '25 12:01 lolrobbe2