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

Tokens in filters do not appear to work #1036

Closed
ratzlaff opened this issue Apr 6, 2018 · 9 comments
Closed

Tokens in filters do not appear to work #1036

ratzlaff opened this issue Apr 6, 2018 · 9 comments

Comments

@ratzlaff
Copy link
Contributor

ratzlaff commented Apr 6, 2018

If I add the following test case to modules/vstudio/tests/cs2005/test_files.lua:

	function suite.copyActionToken()
		_G.testExt="doc"
		files { "Hello.%{testExt}" }
		filter { "files:Hello.%{testExt}" }
			buildaction "Copy"
		prepare()
		test.capture [[
		<Content Include="Hello.doc">
			<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
		</Content>
		]]
		_G.testExt=nil
	end

I get

Running action 'test'...
vstudio_cs2005_files.copyActionToken: expected:
                <Content Include="Hello.doc">
...but was:
                <None Include="Hello.doc" />
fulltext:
                <None Include="Hello.doc" />

I am happy to try and fix this, but I don't yet understand how/when the tokens are expanded, or if the current design of premake allows for this to happen. Any ideas?

@tvandijck
Copy link
Contributor

That is "correct", there is no token expansion happening for filters, and that is not something you can easily fix. The filter function creates a new settings block internally, where the filter terms are compiled into a set of criteria to match whenever the blocks are 'baked'. This sound a little cryptic maybe, I'm sorry if I'm not doing a good job...

Either way... there is a very clear difference in the timing of execution of the filter function compared to anything that is happening within the filter block. Filter 'evaluates' immediately when you call it, the other functions simply store the data you give it, and are 'evaluated' during the baking process..

So, while it is not impossible to expand tokens inside of the filter function, it would be a a very expensive way of doing filter { "files:Hello." .. testExt }, since that is effectively what the token expansion code does... string replacement.

Tokens like %{cfg.buildcfg} or %{prj.name}` wouldn't be available at that time, because those tokens are only available during the bake process.

@ratzlaff
Copy link
Contributor Author

ratzlaff commented Apr 7, 2018

Thanks for the explanation, I think I understand the reasons for immediately evaluating filter expressions.

TLDR

I have been able to enable some file-specific build options using filter { "files: ..." }.
I want to use the same string building logic for files { ... } and filter { "files: ..." }, and I believe that the detoken'ing logic will make my premake scripts more readable. I just don't know (yet) where to poke things in the right place.

Some Context

The premake scripts I am in charge of here at my company use string concatenation, since in our current build of premake5, token support was new and relatively unknown at the time. I am having to update our premake executable and I figure now is a good time to resolve many outstanding deprecation issues. (we sill use configuration, it no longer works when building a VS2015 project from a linux environment)

So in the process of updating everything to use filter, I'm seeing how we concatenate a bunch of strings from different sources to build file and directory paths and its not trivial to read (visually) with all of the .. "/" .. breaking things up.

I could use string.format, but token expansion seems better since it does some checking on the token and seems to make relative paths in (all?) cases. I am cautiously optimistic about not having to manage those relative paths myself.

Finally, when we first started using premake (back on the old industriousone forums), I had hacked in my own function to configure certain files to be compiled with the C++ compiler. My premake hacks are "functional" but limited in scope and not terribly flexible. I have since been able to make compileas work under filter "files: ...". The current plan is to submit this as a PR, but my testing has uncovered this token inconsistency that I think would be a reasonable thing to change (or at the very least, document in the wiki somewhere).

Basically I want to do something like this:

premake.api.register { name = "src", kind = "directory", scope = "project" }
project "main"
   src "src"
   files { "%{prj.src}/**.cpp", "%{prj.src}/**.c" } -- this works
   filter "files:%{prj.src}/module/file.c" }        -- What this issue is about
      compileas "C++"                               -- A future PR

project "plugin"
   src "plugin/src"
   files { "%{prj.src}/**.c" }               -- works
   filter { "files:%{prj.src}/dir/file.c" }  -- What this issue is about
      compileas "C++"                        -- A future PR

@samsinsane
Copy link
Member

I'm fairly certain you can use ** in a filter, so %{prj.src} wouldn't be necessary? filter { "files:**/dir/file.c" } should work, I could be remembering this wrong - we ended up removing all of our file filters since gmake doesn't support them and it's all we had access to at the time.

Also, compileas was added before alpha12 was released, so you should have access to that - assuming your comment is referring to adding it into Premake?

@ratzlaff
Copy link
Contributor Author

ratzlaff commented Apr 8, 2018

** in filters do work, but for some reason that just seems like overkill. If that is going to be the case, I would argue that should be what premake generates for me by default in C++ projects. What do you think?

But then I think the reverse would be true. At some point, someone is going to need a .c file to be compiled with the C compiler when used in a C++ project, so while blanket filter ** rules would fix my use case, I don't think its a one-size fits all solution.

@samsinsane I'm not adding the compileas, I am making it work inside file-filters for xcode, visual studio and gmake projects.

@tdesveauxPKFX
Copy link
Contributor

The default behavior for visual studio is to depend on the file extension. So a .c file will be compiled as c and a .cpp or cxx as c++. The CompileAs tag is for file with a different extension than expected. Like, let's say .test for a file containing c code.

On the makefile side, g++ can compile c code with -x c option. However, both gmake and gmake2 action use $(CC) and not $(CXX) for c files in c++ projects. I guess using the extension?

I do not know how xcode handle all that.

@samsinsane
Copy link
Member

** in filters do work, but for some reason that just seems like overkill. If that is going to be the case, I would argue that should be what premake generates for me by default in C++ projects. What do you think?

If you want all of your C files to compile as C++, you can just as easily specify compileas "C++" at the project level. Premake having defaults for things like this makes it harder to use in situations like what you described above; when one would want to use the C compiler.

I think that ** in the filter is enough in most cases, but I can imagine there's plenty where accessing tokens would almost be required. Unfortunately, as @tvandijck has already pointed out, there's issues accessing them from the filter API.

@tvandijck
Copy link
Contributor

tvandijck commented Apr 9, 2018

As @samsinsane said...

filter "files:%{prj.src}/module/file.c" }

%{prj.src} would be unavailable at that location.

Since you registered src as an API, it's a value stored in blocks, meaning they are evaluated lazily.
filter is not, and the prj variable simply doesn't exits in that scope.

The problem is maybe a little better highlighted in this example.

project 'foobar'
    filter { 'configurations:debug' }
       src './debug'
    filter { 'configurations:release' }
       src './release'

    filter { "files:%{prj.src}/module/file.c" }        -- What this issue is about
       compileas "C++"  

prj.src is simply unknown until we start baking the configurations, so the filter block at the end there cannot be calculated until later, but that is not how filter currently evaluates the filter terms. I guess it would be possible to detoken late (when we compare the criteria's), but I have no real good idea on where to do that right this second, and my fear is that the performance impact of that will be quite severe.

@tvandijck
Copy link
Contributor

So, I think if you wanted to make this work, you have to call the detoken function from here:

https://github.com/premake/premake-core/blob/master/src/host/criteria_matches.c#L293

On both the 'word' and the 'filename' variables.
I have no idea what how you would be able to get to the context/environment at that point however.
potentially you'd have to wrap this method in a lua function, and preprocess the data before entering it, but you have to do it is a non-destructive way, so you're going to end up copying the criteria tables for every call to criteria.matches..

@ratzlaff
Copy link
Contributor Author

ratzlaff commented Apr 9, 2018

Ah, that is a better example illustrating how things operate. I have edited the Tokens page with a link to your comment.
Thanks for the hint of where to start!

I'll close this for now, since this does not appear to be a simple thing to refactor.

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

No branches or pull requests

4 participants