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

Enable msbuild batching for custom rules

Open cathei opened this issue 1 year ago • 6 comments

What does this PR do?

This PR lets users to write custom rules compatible with msbuild batching. By providing @(Rule) as input and @(Rule->'%(Output)') as output, msbuild can recognize this task can be batched and built incrementally.

How does this PR change Premake's behavior?

Existing rules should be unaffected because the behavior should be same when using tokens like ${file.relpath} since properties like %(Identity) or %(FullPath) breaks batching.

To enable batching, user should write command with newly added tokens %{rule.inputs} or %{rule.outputs} which mapped to string list. For non-msbuild environments like gmake2 or codelite, these tokens will be substituted as single input and output as there're no batching.

Anything else we should know?

Let me know if introducing new tokens is undesirable.

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
  • [x] 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!

cathei avatar Sep 18 '23 00:09 cathei

So I went with fcfg_mt.ruleinputs as fallback, which just returns fcfg.relpath. On supported platform (MSBuild only for now) this should expand to proper list of inputs (depends on build system). As a result there's no modification needed for other existing modules.

I excluded output list token for now, as it's bit complex to get value and users probably can deduce it from inputs. Can be improved in future.

Also docs are updated as reviewed.

cathei avatar Sep 22 '23 05:09 cathei

This PR lets users to write custom rules compatible with msbuild batching. By providing @(Rule) as input and @(Rule->'%(Output)') as output, msbuild can recognize this task can be batched and built incrementally.

Unclear for me what it means... From the link I quickly read, it seems more a code factorization. and as you write the rule in Premake, it is not really a concern. (not even sure the factorization takes place with generated code).

What do you mean by "can be batched and built incrementally"? as I understand inputs/ouputs are the important part for individual build, not the buildcommands...

Jarod42 avatar Sep 22 '23 14:09 Jarod42

What do you mean by "can be batched and built incrementally"?

I'll give brief example. When you have custom rule,

rule 'MyCustomRule'
  fileextensions { '.txt' }
  buildcommands { 'echo %{file.ruleinputs}'  } -- using new ruleinputs token

Without batching msbuild will invoke each command per file separately.

echo a.txt
echo b.txt
echo c.txt

When batching is possible, msbuild can invoke command as a whole. (Incremental just means that if only a.txt changed only echo a.txt will be executed unless it's clean build.)

echo a.txt b.txt c.txt

This can boost performance for the commands that have long startup time or use shared resorces.

cathei avatar Sep 23 '23 01:09 cathei

OK, so your code is a shorthand for:

rule "myrule"
	--location(LocationDir)
	--display "My custom rule"
	fileextension ".txt"

	-- propertydefinition { .. }
	-- buildinputs { "%{file.relpath}.in" } -- Possible extra inputs
	-- buildoutputs { path.join(LocationDir, "%{file.basename}.out") } -- All outputs
if "vs2000" <= _ACTION and _ACTION <= "vs3000" then -- Batch only handled by vs*
	buildmessage 'custom rule: [Inputs]'
	buildcommands { "MyCommand [Inputs]" }
else
	buildmessage 'custom rule: %{file.relpath}'
	buildcommands { "MyCommand %{file.relpath}" }
end
-- above if-block replaced by
--	buildmessage 'custom rule: %{file.ruleinputs}'
--	buildcommands { "MyCommand %{file.ruleinputs}" }

%{file.ruleinputs} seems a wrong naming: it is the name of several %{file}. rule.inputs is better IMO (but might be confused with buildinputs though (as I did several post above)).

Wonder if a flag batchbuild "on" would be better in that case.

Jarod42 avatar Sep 23 '23 13:09 Jarod42

OK, so your code is a shorthand for:

Yes pretty much.

%{file.ruleinputs} seems a wrong naming: it is the name of several %{file}. rule.inputs is better IMO (but might be confused with buildinputs though (as I did several post above)).

I agree that rule.inputs is better in terms of naming, but I can't find any good customization point like fcfg_mt without making seemingly big change that could require external modules to update.

Wonder if a flag batchbuild "on" would be better in that case.

I think it is better to be separate token, since even if there is flag, many tokens can break batch if MSBuild can't detect common pattern.

cathei avatar Sep 23 '23 16:09 cathei

Do you mind pushing an empty commit just to get the actions to trigger? I think I'm good to merge it, assuming all tests pass.

nickclark2016 avatar Dec 05 '23 13:12 nickclark2016