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

"location" command clarification #564

Closed
csobaistvan opened this issue Aug 10, 2016 · 5 comments
Closed

"location" command clarification #564

csobaistvan opened this issue Aug 10, 2016 · 5 comments

Comments

@csobaistvan
Copy link

csobaistvan commented Aug 10, 2016

So today I came back to the premake5 repository to see how development goes, because I just love this project & build system. I quickly noticed that the first official tutorial is up, so I thought I'd check it out. Then I thought I found a bug in it - it only had a workspace level "location" call, yet it claimed that all generated files would be put into the "Generated" folder. Quoting the wiki page for location:

Note that unlike other values, location does not automatically propagate to the contained projects. Projects will use their default location unless explicitly overridden.

So my question is: is the wiki page for location deprecated/wrong, or is it a bug? Either way, one or the other should be fixed. I suspect it's the wiki page, but I don't want to go ahead and modify it without knowing for sure that this is the intended behavior.

@tvandijck
Copy link
Contributor

Well, that is certainly not the behavior I'm seeing indeed... and also not what the code does:

this is in the oven.project.bake()

self.location = self.location or wks.location or self.basedir

So it is true that if the project.location is set it override it, and if the workspace.location is not set it will have some kind of default... but the documentation is incorrect on it not propagating.

@samsinsane
Copy link
Member

samsinsane commented Nov 3, 2016

Related, the documentation also says:

By default, workspace and project files are generated into the same directory as the script that defines them.

Which is broken because the value propagates and wks.location is defined by:

self.location = self.location or self.basedir

And self.basedir is defined by:

self.basedir = os.getcwd()

I went back through the history and I can't see where the described functionality would ever work. self.basedir was defined as os.getcwd() as far back as 2009, I'm sure something, somewhere, changed along the way that has caused this to start happening. Any ideas on how we can achieve the documented functionality and not have self.location be nil? It looks like a bit of code relies on the various locations to be defined.

This issue was raised in #371.

@starkos
Copy link
Member

starkos commented Nov 3, 2016

I went back through the history and I can't see where the described functionality would ever work. self.basedir was defined as os.getcwd() as far back as 2009, I'm sure something, somewhere, changed along the way that has caused this to start happening.

It works because Premake's implementation of luaL_loadfile() changes to the directory containing the script before running it. So when those snippets of code are executed, the current directory is in fact the one containing the script.

That said, we now have the _SCRIPT_DIR global, which would be a more reliable source for that path.

@samsinsane
Copy link
Member

It works because Premake's implementation of luaL_loadfile() changes to the directory containing the script before running it. So when those snippets of code are executed, the current directory is in fact the one containing the script.

That's not quite what I meant, the code that has been posted above has never changed from what I could tell when I went through the history of the repo. So how does self.location = self.location or wks.location or self.basedir ever set self.location to self.basedir? wks.location is always set.

Just to clarify, do you experience this bug too? I think I've always had this bug :(

@samsinsane
Copy link
Member

This has been fixed by #733

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