Skip to content

Add Shell Execution Extensions to Context #4

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

AristurtleDev
Copy link
Contributor

Adds extension methods for FrostingContext to help with executing shell commands per platform

{
private static readonly ProcessSettings _processSettings;

static FrostingContextExtensions() => _processSettings = new ProcessSettings();
Copy link
Member

Choose a reason for hiding this comment

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

Just do:

private static readonly ProcessSettings _processSettings = new();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 99399d0


public static void SetShellWorkingDir(this FrostingContext context, string path)
{
_processSettings.WorkingDirectory = path;
Copy link
Member

Choose a reason for hiding this comment

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

Feel free to use the => thingy here if you want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 99399d0

_processSettings.WorkingDirectory = path;
}

public static int ShellExecute(this FrostingContext context, string command, string environmentVariables = "")
Copy link
Member

@harry-cpp harry-cpp Apr 29, 2024

Choose a reason for hiding this comment

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

Pass env variables in a separate function SetShellEnvVariables(parms string[] env)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 23d2fc1

From our previous discussion, the export keyword is in fact required. Without it, the environment variables do not stay set if the command executes something that in turn executes something else. So the export is required for the entire run of the process.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Additionally, I was not able to find any decent way of doing space character escapes in the command. The problem comes in situations where you want to set an environment variable such as

context.SetShellEnvironmentVariables("CFLAGS", $"-w -arch arm64 -I{dependencyDir}/include");

The space escapes would need to be set for the path in the -I flag, however you would need to differentiate between what is a path and what isn't, such as the -w -arch arm64 part. Otherwise, you end up with a sanitized environment variable such as CFLAGS="-w\ -arch\ arm64\ -i/path/to\ some/directory/include;" which fails to run.

Open to suggestions on this if still needed

Copy link
Member

@harry-cpp harry-cpp Apr 30, 2024

Choose a reason for hiding this comment

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

You are supposed to set it to either:

CFLAGS=-w\ -arch\ arm64\ -i/path/to\ some/directory/include;

or

CFLAGS="-w -arch arm64 -i/path/to some/directory/include";

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.

2 participants