Read and parse agent config

[ofalvai]

Checklist

• I've read and followed the Contribution Guidelines
• README.md is updated with the changes (if needed)

Version

Requires a MINOR version update

Context

First PR of a series of changes preparing Bitrise CLI for agent-like persistent behavior. The initial features are:

• override special Bitrise directories for all builds served by the CLI on a given host
• provide hooks for cleaning up Bitrise directories at the start/end of the build

Changes

• Define the new agent config file
• Read and parse this new file, including env var expansion and shell tilde expansion where it makes sense

Investigation details

Decisions

The config field names are not final, we might rename them later.

[lpusok]

Should check if the hook files exist at some point, maybe here?

[ofalvai]

Yes, good point, I'm on it

[ofalvai]

Take a look now @lpusok

[tothszabi]

These are kinda duplicated a bit:

bitrise/configs/paths.go

Lines 28 to 43 in 604afd8

	const (
	// EnvstorePathEnvKey ...
	EnvstorePathEnvKey = "ENVMAN_ENVSTORE_PATH"
	// FormattedOutputPathEnvKey ...
	FormattedOutputPathEnvKey = "BITRISE_STEP_FORMATTED_OUTPUT_FILE_PATH"
	// BitriseSourceDirEnvKey ...
	BitriseSourceDirEnvKey = "BITRISE_SOURCE_DIR"
	// BitriseDeployDirEnvKey ...
	BitriseDeployDirEnvKey = "BITRISE_DEPLOY_DIR"
	// BitriseTestDeployDirEnvKey is the root directory of test reports
	BitriseTestDeployDirEnvKey = "BITRISE_TEST_DEPLOY_DIR"
	// BitrisePerStepTestResultDirEnvKey is a unique subdirectory in BITRISE_TEST_DEPLOY_DIR for each step run, steps should place test reports and attachments into this directory
	BitrisePerStepTestResultDirEnvKey = "BITRISE_TEST_RESULT_DIR"
	// BitriseTmpDirEnvKey ...
	BitriseTmpDirEnvKey = "BITRISE_TMP_DIR"
	)

I know they are not exactly the same thing but I was wondering if we could unify them. I could see any meaningful way apart from linking the two together with some explanation in a comment.

But this made me thinking a but further. Like what will happen if we introduce a new folder? We need to add it here and the user needs to add it to their agent config. If they do not do that then our step might not work if properly with a non empty folder.

Then I looked at the example comfig and found this piece of code a bit repetitive:

BITRISE_SOURCE_DIR: /opt/bitrise/workspace/$BITRISE_APP_SLUG
BITRISE_DEPLOY_DIR: /opt/bitrise/$BITRISE_APP_SLUG/$BITRISE_BUILD_SLUG/artifacts
BITRISE_TEST_DEPLOY_DIR: /opt/bitrise/$BITRISE_APP_SLUG/$BITRISE_BUILD_SLUG/test_results

which made me think that would not it be simpler if the user just needs to specify a single home folder and the CLI would create the subdirectories using a specific pattern? That would help us to introduce new folders without user action.
So in this example the user would only need to specify this:

BITRISE_DATA_HOME: /opt/bitrise/workspace

and then the CLI would automatically create and export the following paths:

BITRISE_SOURCE_DIR: $BITRISE_DATA_HOME/$BITRISE_APP_SLUG
BITRISE_DEPLOY_DIR: $BITRISE_DATA_HOME/$BITRISE_APP_SLUG/$BITRISE_BUILD_SLUG/artifacts
BITRISE_TEST_DEPLOY_DIR: $BITRISE_DATA_HOME/$BITRISE_APP_SLUG/$BITRISE_BUILD_SLUG/test_results

[ofalvai]

Yeah, I also think it would be cleaner to have everything in one folder, but I feel that it's a bit risky to change these things now. There are two existing behaviors that this would change:

• On the CI stacks, we define unique directories as env vars for each Bitrise dir
• The bitrise run command works in a way that if these env vars are not defined (such as when running locally), it creates a temporary dir in $TMPDIR for each Bitrise dir

Also, because of the second behavior, I wouldn't worry about adding new Bitrise dirs and existing agents not defining an override. The CLI is going to create a temp dir at runtime, so even though it's not going to get cleaned up, it's also not going to cause conflicts between builds.

[ofalvai]

Also, the test file in the PR is just one example of configuring the paths. For example, this source dir config:

BITRISE_SOURCE_DIR: $BITRISE_DATA_HOME/$BITRISE_APP_SLUG

...will do a clean git clone in every build. Most agent setups would probably want to use a static folder for all builds to save on the clone time. If we "derive" the source dir path automatically from BITRISE_DATA_HOME, users wouldn't be able to choose between the two use cases.

[tothszabi]

I did not spend as much time as you on this topic so probably that is why I do not see why these are issues. So if you are happy with it then I am happy with it.

What I would say is that I would not be afraid to change / introduce a better behaviour if needed. We are introducing something brand new so we have the opportunity to do whatever cleanup / improvement we want.

I can see one problem with the CLI creating a temp dir for the undefined folders. As far as I know (this might be outdated now) the OS cleans up the temp dir when it boots. If that is true and the CLI creates a temp dir in every build then because these agents (potentially) never restart we might fill up the hard drive over time. I am not saying it will happen but definitely will have the possibility with that behaviour.

.will do a clean git clone in every build

I do not fully understand this part. In my example the BITRISE_SOURCE_DIR: $BITRISE_DATA_HOME/$BITRISE_APP_SLUG is static path because the BITRISE_DATA_HOME is defined as /opt/bitrise/workspace where nothing is machine specific. The app slug also never changes. Based on these this path should not change.

users wouldn't be able to choose between the two use cases

We can have both. We only need a mandatory BITRISE_DATA_HOME and next to it we could have the possibility for the users to override every other path if they wish. But at least with this we could have a main home folder which we could use as fallback.

[ofalvai]

We can have both. We only need a mandatory BITRISE_DATA_HOME and next to it we could have the possibility for the users to override every other path if they wish. But at least with this we could have a main home folder which we could use as fallback.

Oh, I like this approach, thank you for the idea. I implemented it in the latest commit, what do you think?

Regarding the risky change, I meant the risk of breaking the stacks with a new interaction between the CLI and the env vars 😄

I do not fully understand this part. In my example the BITRISE_SOURCE_DIR: $BITRISE_DATA_HOME/$BITRISE_APP_SLUG

You are right, I overlooked that the app slug segment of the path is static, I wanted to argue that if the build slug is also included in the path (which we expect some users to do)

[tothszabi]

I like what you introduced in your latest commits. 👍🏻

[tothszabi]

I guess this can be removed.