Enable msbuild batching for custom rules #2141

cathei · 2023-09-18T00:26:36Z

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?

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!

add `rule.inputs` and `rule.outputs` pathVars add test cases add documentation

nickclark2016 · 2023-09-18T03:04:03Z

website/docs/Custom-Rules.md

+
+## Rule Batching
+
+With msbuild, custom rules can be batched if same properties are used. To enable this, use `rule.inputs` or `rule.outputs` tokens in `buildcommands`. If corresponding output is up-to-date to the input, then the input will be omitted.


Nit: Rename msbuild to MSBuild

Jarod42 · 2023-09-18T14:02:41Z

modules/gmake2/_preload.lua

+
+		pathVars = {
+			["rule.inputs"]	 = { absolute = false, token = "$<" },
+			["rule.outputs"] = { absolute = false, token = "$@" }


For non-msbuild environments like gmake2 or codelite, these tokens will be substituted as single input and output as there're no batching.

What would be the value without pathVars ? (i.e for gmake/xcode/external modules)
Does they need update?

Those tokens are only valid in buildcommands in custom rule context.
For built-in modules it seems like gmake2, codelite and vs are only modules support custom rules.

However I do think it's better to have fallback for any modules. I was thinking of using fcfg_mt for fallback and name tokens something like file.ruleinputs/file.ruleoutputs. Does that make more sense?

Custom rules are indeed not supported by gmake and xcode.
Some external module such as codeblocks/ninja does support it.

How is rule.outputs "concatenated"? with space? (and $@ $< may probably be removed).
(first token which is an array...)
rule.inputs is a little more complex, Does it include %{file.relpath}?

Yes, in VS modules,
rule.inputs is space-concatenated file.relpaths for the batch. (Same as item included in .vcxproject)
rule.outputs is space-concatenated path.normalize(buildoutputs)s for the batch. (Same as outputs defined in .targets)

Jarod42 · 2023-09-18T14:03:53Z

website/docs/Custom-Rules.md

@@ -72,3 +72,14 @@ project "MyProject"
      StripDebugInfo = true
    }
 ```
+
+## Rule Batching


Those tokens should also go in Tokens.md IMO.

cathei · 2023-09-22T05:10:37Z

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.

Jarod42 · 2023-09-22T14:22:35Z

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...

cathei · 2023-09-23T01:00:53Z

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.

Jarod42 · 2023-09-23T13:23:28Z

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.

cathei · 2023-09-23T16:52:27Z

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.

nickclark2016 · 2023-12-05T13:27:51Z

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.

Enable msbuild batching for custom rules

0027213

add `rule.inputs` and `rule.outputs` pathVars add test cases add documentation

nickclark2016 reviewed Sep 18, 2023

View reviewed changes

Jarod42 reviewed Sep 18, 2023

View reviewed changes

renaming token to file.ruleinputs, excluding outputs for now.

21112c7

nickclark2016 approved these changes Dec 5, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable msbuild batching for custom rules #2141

Enable msbuild batching for custom rules #2141

cathei commented Sep 18, 2023 •

edited

Loading

nickclark2016 Sep 18, 2023

Jarod42 Sep 18, 2023

cathei Sep 19, 2023

Jarod42 Sep 19, 2023 •

edited

Loading

cathei Sep 20, 2023

Jarod42 Sep 18, 2023

cathei commented Sep 22, 2023 •

edited

Loading

Jarod42 commented Sep 22, 2023

cathei commented Sep 23, 2023 •

edited

Loading

Jarod42 commented Sep 23, 2023 •

edited

Loading

cathei commented Sep 23, 2023

nickclark2016 commented Dec 5, 2023


		## Rule Batching

		With msbuild, custom rules can be batched if same properties are used. To enable this, use `rule.inputs` or `rule.outputs` tokens in `buildcommands`. If corresponding output is up-to-date to the input, then the input will be omitted.

Enable msbuild batching for custom rules #2141

Are you sure you want to change the base?

Enable msbuild batching for custom rules #2141

Conversation

cathei commented Sep 18, 2023 • edited Loading

nickclark2016 Sep 18, 2023

Choose a reason for hiding this comment

Jarod42 Sep 18, 2023

Choose a reason for hiding this comment

cathei Sep 19, 2023

Choose a reason for hiding this comment

Jarod42 Sep 19, 2023 • edited Loading

Choose a reason for hiding this comment

cathei Sep 20, 2023

Choose a reason for hiding this comment

Jarod42 Sep 18, 2023

Choose a reason for hiding this comment

cathei commented Sep 22, 2023 • edited Loading

Jarod42 commented Sep 22, 2023

cathei commented Sep 23, 2023 • edited Loading

Jarod42 commented Sep 23, 2023 • edited Loading

cathei commented Sep 23, 2023

nickclark2016 commented Dec 5, 2023

cathei commented Sep 18, 2023 •

edited

Loading

Jarod42 Sep 19, 2023 •

edited

Loading

cathei commented Sep 22, 2023 •

edited

Loading

cathei commented Sep 23, 2023 •

edited

Loading

Jarod42 commented Sep 23, 2023 •

edited

Loading