Skip to content

Pass through shell script build phase environment and working directory for build tool commands #3333

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

Conversation

abertelrud
Copy link
Contributor

Use the new LLBuildManifestWriter support to pass through shell script build phase environment and working directory for build tool commands.

This PR builds on #3332

Motivation:

Plugin-created commands should be able to specify environment and initial working directory for those commands.

Modifications:

rdar://74663489

@abertelrud abertelrud changed the title Pass through shell script build phase environment and working directory for build tool commands.\ Pass through shell script build phase environment and working directory for build tool commands Mar 8, 2021
@abertelrud
Copy link
Contributor Author

@swift-ci please smoke test

@abertelrud
Copy link
Contributor Author

Note: This should only be merged after #3332 since every PR is a squash now but the changes should be separate in the revision history.

@abertelrud
Copy link
Contributor Author

abertelrud commented Mar 8, 2021

error: command Generating foo.swift from foo.dat failed: working-directory unsupported on this platform

Ok, that's disappointing. That will require an llbuild fix.

@abertelrud
Copy link
Contributor Author

Oh right, it's because it uses pthread_chdir_np() which is only supported on Darwin. In this case we can probably work around it by wrapping it in a shell and prepending a cd.

@abertelrud
Copy link
Contributor Author

abertelrud commented Mar 8, 2021

It's a little annoying — supporting initial working directory is mostly for completeness. Build tools should in general take full paths and not rely on a custom working directory. We'll need to decide whether supporting relative paths (arguably an anti-pattern) is important enough to put in workarounds, or whether it's better to leave it out and just have the plugin form the path.

Environment is the important one to support.

@tomerd tomerd added the ready Author believes the PR is ready to be merged & any feedback has been addressed label Mar 8, 2021
@abertelrud
Copy link
Contributor Author

I think what I'm going to do here is to remove the working directory functionality for now. It's not so important as to steal time from other things (such as better diagnostics, etc), and it is straightforward to add later.

@abertelrud abertelrud removed the ready Author believes the PR is ready to be merged & any feedback has been addressed label Mar 8, 2021
@tomerd tomerd added the ready Author believes the PR is ready to be merged & any feedback has been addressed label Mar 8, 2021
@tomerd tomerd removed the ready Author believes the PR is ready to be merged & any feedback has been addressed label Mar 8, 2021
@abertelrud abertelrud marked this pull request as draft March 8, 2021 21:46
@artemcm
Copy link
Contributor

artemcm commented Mar 9, 2021

@swift-ci please smoke test linux

…t build phase environment and working directory for build tool commands.

Support for working directories is temporarily marked as unavailable since llbuild doesn't currently support it on platforms other than Darwin.  This restriction will be removed when llbuild starts to support it.

rdar://74663489
@abertelrud abertelrud force-pushed the plugin-buildtool-cmds-environ-and-workdir branch from f009b72 to 7ee3551 Compare March 10, 2021 02:27
@abertelrud
Copy link
Contributor Author

Updated to mark the ability to set a working directory as not yet available, and updated unit test.

@abertelrud
Copy link
Contributor Author

@swift-ci please smoke test

@abertelrud abertelrud marked this pull request as ready for review March 10, 2021 02:32
@abertelrud abertelrud added the ready Author believes the PR is ready to be merged & any feedback has been addressed label Mar 10, 2021
@abertelrud abertelrud merged commit 85f81f3 into swiftlang:main Mar 10, 2021
@abertelrud abertelrud deleted the plugin-buildtool-cmds-environ-and-workdir branch March 10, 2021 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready Author believes the PR is ready to be merged & any feedback has been addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants