Reference paths relative to project root #1800
Conversation
Before this PR: - some scripts change the current working directory and use relative paths - different approaches are taken to know which directory a script is running in - paths are sometimes relative, sometimes absolute, sometimes traversing directories After this PR: - scripts do neither change nor care much about the current working directory - a unified approach determines the directory of the current script - paths are always relative to the project root This should resolve an issue I already tried to fix with #1798, where the contents of `./sentry` were not copied into the built container image, thus `enhance-image.sh` did not apply.
|
Thanks for your consideration. I wanted to make sure this is a direction you would like to pursue before doing further work on this. Can you assist me with the failing test cases? I am having trouble figuring out what is going wrong in |
Co-authored-by: Amin Vakil <info@aminvakil.com>
spawnia
changed the title
|
/gcbrun |
|
Hm, not sure why Google Cloud build is failing, I'll look into it. |
…erences # Conflicts: # _unit-test/error-handling-test.sh # install/build-docker-images.sh # install/error-handling.sh
|
@ethanhs @hubertdeng123 I guess CI is only failing due to #1803 (comment)? Given everything else passes, can you merge regardless so this fix can move forward? |
|
I'll take another look at this in a little bit, and if it looks good merge things. |
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you label it "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
I still think this is relevant and necessary and hope to see it merged some time soon. |
…erences # Conflicts: # _unit-test/error-handling-test.sh # install/build-docker-images.sh
|
@ethanhs @hubertdeng123 I have now updated this branch with the latest changes from |
There was a problem hiding this comment.
Sorry for the late response @spawnia , did a re-review here. I'm pretty neutral on these changes here but seems generally fine
It also looks like the permissions of detect-platform.sh has been changed.
Yup, this is intentional. The file is only able to be sourced from other scripts. Due to lack of a shebang and the nature of its contents, it would not be able to be executed by itself. |
|
Thank you @hubertdeng123 |
|
Sorry for the long delay here @spawnia, thanks for submitting this PR 😁 |
Before this PR:
After this PR:
This should resolve an issue I already tried to fix with #1798, where the contents of
./sentrywere not copiedinto the built container image,
thus
enhance-image.shdid not apply.Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.