Skip to content

Fix issue running scons using a symlink to scons.py in an scons-local dir #3271

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

Merged
merged 3 commits into from
Mar 12, 2019

Conversation

mwichmann
Copy link
Collaborator

Earlier fix to running with scons-local when the directory is a
symlink (i.e. scons-local symlinked to scons-local-{versiontag} and
invoked as scons-local/scons.py) broke running scons when the
"executable" is a symlink. Restoring that behavior.

This is dodgy to test reliably on a developer system, as the
fallback mechansim to find a usable scons goes on to look for
a system-installed scons, so it looked like it was working. I
will add a test if I can figure out a good way.

Signed-off-by: Mats Wichmann mats@linux.com

Contributor Checklist:

  • I have created a new test or updated the unit tests to cover the new/changed functionality.
  • I have updated master/src/CHANGES.txt directory (and read the README.txt in that directory)
  • I have updated the appropriate documentation

@@ -14,11 +14,14 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER
- Improve finding of Microsoft compiler: add a 'products' wildcard
in case 2017 Build Tools only is installed as it is considered a separate
product from the default Visual Studio
- scons.py and sconsign.py stopped working if script called as a symlink
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to "Fixed issue running scons-local when called via a symlink"

@coveralls
Copy link

Coverage Status

Coverage increased (+1.7%) to 81.641% when pulling 1f59bd9 on mwichmann:scons-symlink into f3739c4 on SCons:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+1.7%) to 81.641% when pulling 1f59bd9 on mwichmann:scons-symlink into f3739c4 on SCons:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.7%) to 81.641% when pulling 1f59bd9 on mwichmann:scons-symlink into f3739c4 on SCons:master.

@coveralls
Copy link

coveralls commented Jan 15, 2019

Coverage Status

Coverage decreased (-0.3%) to 80.357% when pulling 74187aa on mwichmann:scons-symlink into ca839c4 on SCons:master.

src/CHANGES.txt Outdated
@@ -14,11 +14,14 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER
- Improve finding of Microsoft compiler: add a 'products' wildcard
in case 2017 Build Tools only is installed as it is considered a separate
product from the default Visual Studio
- Fixed issue running scons-local using a symlink to the scons.py in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh is there and issue # for this, if so please reference?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the report came in on irc and I don't see a GH issue for it.

Earlier fix to running with scons-local when the directory is a
symlink (i.e. scons-local symlinked to scons-local-{versiontag} and
invoked as scons-local/scons.py) broke running scons when the
"executable" is a symlink. Restoring that behavior.

This is dodgy to test reliably on a developer system, as the
fallback mechansim to find a usable scons goes on to look for
a system-installed scons, so it looked like it was working. I
will add a test if I can figure out a good way.

Signed-off-by: Mats Wichmann <mats@linux.com>
@mwichmann mwichmann changed the title scons-local version has symlink problems Fix issue running scons using a symlink to scons.py in an scons-local dir Jan 19, 2019
Signed-off-by: Mats Wichmann <mats@linux.com>
@bdbaddog
Copy link
Contributor

bdbaddog commented Feb 2, 2019

Can you make a test for this so we don't break it again going forward?

@bdbaddog bdbaddog merged commit f0f1dd1 into SCons:master Mar 12, 2019
@mwichmann mwichmann deleted the scons-symlink branch March 12, 2019 21:20
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.

3 participants