-
-
Notifications
You must be signed in to change notification settings - Fork 228
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
Don't wastefully execute everything via a shell #2674
Conversation
✅ PR OK, no changes in deprecations or warnings Total deprecations: 15 Total warnings: 0 Build statistics: statistics (-before, +after)
-executable size=5284832 bin/dub
-rough build time=77s
+executable size=5280736 bin/dub
+rough build time=76s Full build output
|
can you rebase to get the CI fixes in? |
We should only ever use the shell if we want to use the shell's features, such as pipes, non-trivial redirection, or shell language features (branches/loops/etc.). Otherwise, executing a program via the shell is pointless and wasteful.
20c5e39
to
0eeed18
Compare
Sure, done. |
Failure mode is different as we're no longer asking the shell to run the dummy program, we're doing it ourselves.
@@ -61,7 +61,7 @@ fi | |||
rm ../etc/dub/settings.json | |||
echo "Empty file named ldc2." > ../bin/ldc2 | |||
|
|||
if ! { ${DUB} describe --single issue103-single-file-package.d 2>&1 || true; } | grep -cF "Failed to invoke the compiler $(dirname $CURR_DIR)/bin/ldc2 to determine the build platform"; then | |||
if ! { ${DUB} describe --single issue103-single-file-package.d 2>&1 || true; } | grep -cF "Failed to execute '$(dirname $CURR_DIR)/bin/ldc2'"; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uh do you know why this failed? / needed to be changed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes? I explained it in the commit message.
CyberShadow/DAutoTest stuck? |
It's running right now. |
We should only ever use the shell if we want to use the shell's features, such as pipes, non-trivial redirection, or shell language features (branches/loops/etc.). Otherwise, executing a program via the shell is pointless and wasteful.