Skip to content

Conversation

@tritao
Copy link
Contributor

@tritao tritao commented Oct 11, 2024

What does this PR do?

Updates the code to use _SCRIPT_DIR instead of os.getcwd().

As alluded to by @starkos, it's a more reliable/clearer way to get the script path: #564 (comment)

How does this PR change Premake's behavior?

None intended.

@tritao tritao force-pushed the script-dir branch 4 times, most recently from 19d3514 to 55c8dca Compare October 13, 2024 01:13
@tritao tritao marked this pull request as ready for review October 13, 2024 01:20
@tritao
Copy link
Contributor Author

tritao commented Oct 13, 2024

Got this work, which was a bit more work than I initially thought it would be... and the main issue is the expectation that all test paths are not specified against their respective script locations but against the root.

Overall I do think using _SCRIPT_DIR vs. os.getcwd is a bit cleaner (and more robust if the user changes working directory mid-script for some reason), but to get the tests to work, I had to point _SCRIPT_DIR to the main repository dir.

The alternatives to that are either changing all test paths to always be relative to their own scripts, or maybe some clever use of basedir or a new _BASE_DIR, which I think could be useful in its own right for user projects (see #2129).

Waiting until I hear the maintainer thoughts to figure out the best direction for this.

@tritao tritao force-pushed the script-dir branch 4 times, most recently from b315c2d to d6f542c Compare October 14, 2024 22:45
@tritao
Copy link
Contributor Author

tritao commented Oct 14, 2024

Got this work, which was a bit more work than I initially thought it would be... and the main issue is the expectation that all test paths are not specified against their respective script locations but against the root.

Overall I do think using _SCRIPT_DIR vs. os.getcwd is a bit cleaner (and more robust if the user changes working directory mid-script for some reason), but to get the tests to work, I had to point _SCRIPT_DIR to the main repository dir.

The alternatives to that are either changing all test paths to always be relative to their own scripts, or maybe some clever use of basedir or a new _BASE_DIR, which I think could be useful in its own right for user projects (see #2129).

Waiting until I hear the maintainer thoughts to figure out the best direction for this.

I went ahead and implemented the solution suggested here. In the end the change turned out to be relatively simple, even if it took a while to get there. With the latest pushed approach there are:

  • No more current working directory shenanigangs going on
  • _SCRIPT_DIR always points to the current script directory
  • New _BASE_DIR internal API that can be used by the test suite runner to set up a base directory path (this could be replaced with some usage of basedir() but its cleaner like this IMHO)
  • Only minor changes to the testsuite to clean up PCH tests which were changing their own working directory

@tritao tritao changed the title Use _SCRIPT_DIR instead of os.getcwd(). Use base dir instead of os.getcwd(). Oct 14, 2024
@tritao
Copy link
Contributor Author

tritao commented Oct 16, 2024

Still not too happy with this approach, I've managed to make a cleaner implementation, will send as another PR.

@tritao tritao closed this Oct 16, 2024
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

Successfully merging this pull request may close these issues.

1 participant