-
Notifications
You must be signed in to change notification settings - Fork 161
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
Add spec for Standardized CI Environment Variables #19
Conversation
@@ -23,7 +23,7 @@ It is important that these environment variables do not conflict with other vari | |||
|
|||
## Support from CI Services | |||
|
|||
This plan will only work if CI services decide to support these environment variables. An important question is whether CI services have similar environment variables already. The table below suggests that the information we need is already available. |
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.
Here are the AWS CodeBuild environment variables for an extra data point. We don't have one for STANDARDCI_REPOSITORYNAME.
STANDARDCI_REPOSITORYCOMMITID = CODEBUILD_RESOLVED_SOURCE_VERSION
STANDARDCI_REPOSITORYROOT = CODEBUILD_SRC_DIR
STANDARDCI_REPOSITORYNAME =
STANDARDCI_REPOSITORYURI = CODEBUILD_SOURCE_REPO_URL
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.
Thanks. Would you be amenable to exposing these new names and also filling the gap on the missing environment variable?
You can add a new column to the table if you'd like, via PR.
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.
I can discuss the option to support them with the AWS CodeBuild team but I would need to know more what they would be used for to convince them.
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.
Makes sense. We're on the cusp of publishing the design that produced this proposal. That should help.
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.
Looking at these names again I'd suggest we insert some more _
's to make them more readable:
STANDARDCI_REPOSITORY_COMMIT_ID
STANDARDCI_REPOSITORY_ROOT
STANDARDCI_REPOSITORY_NAME
STANDARDCI_REPOSITORY_URI
|
||
The .NET Core SDK is implementing new scenarios that require source control manager information. An initial idea was to call out to specific source control tools to get this information. This approach is problematic since it requires an implementation for each source control manager and it has the potential to be fragile and slow. | ||
|
||
It turns out that CI services all of the information that these new .NET Core scenarios need via environment variables. Also, there is a strong relationship between these scenarios and official CI-provided builds (much less on local developer builds). As a result, relying on the CI-provided environment variables is attractive. |
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.
typo: that CI services all of the -> that CI services provide all of the
|
||
The .NET Core SDK needs are oriented around source control. As a result, the intial list is source control oriented, but there is no affinity to source control on the general idea of standardized environment variables. | ||
|
||
It is important that these environment variables do not conflict with other variables. To avoid that, all environment variables will be prepended with "STANDARDCI-". This name is a first proposal for the idea and it may get changed based on feedback. |
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.
should be STANDARDCI_
not STANDARDCI-
|
||
* **STANDARDCI\_REPOSITORYCOMMITID** -- Commit hash / ID; Example: 2ba93796dcf132de447886d4d634414ee8cb069d | ||
* **STANDARDCI\_REPOSITORYROOT** -- Root of repository; Example: D:\repos\corefx | ||
* **STANDARDCI\_REPOSITORYNAME** -- Name repository; Example: dotnet\corefx |
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.
typo: Name repository -> Name of repository
|STANDARDCI\_REPOSITORYCOMMITID | BUILD\_SOURCEVERSION | TRAVIS\_COMMIT |APPVEYOR\_REPO\_COMMIT | CIRCLE\_SHA1 | | ||
|STANDARDCI\_REPOSITORYROOT|BUILD\_REPOSITORY\_LOCALPATH|TRAVIS\_BUILD\_DIR| APPVEYOR\_BUILD\_FOLDER | CIRCLE\_WORKING\_DIRECTORY | | ||
|STANDARDCI\_REPOSITORYNAME|BUILD\_REPOSITORY\_NAME| TRAVIS\_REPO\_SLUG | APPVEYOR\_REPO\_NAME | CIRCLE\_PROJECT\_USERNAME + CIRCLE\_PROJECT\_REPONAME | | ||
|STANDARDCI\_REPOSITORYURI|BUILD\_REPOSITORY\_URI| | | CIRCLE\_REPOSITORY\_URL | |
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.
Travis CI only supports GitHub at the moment so you could assume STANDARDCI_REPOSITORYURI = "https://github.com/" + $TRAVIS_REPO_SLUG
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.
I'm joining the conversation.
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.
@akoeplinger We would like to avoid CI-specific assumptions in the SDK.
It is important that these environment variables do not conflict with other variables. To avoid that, all environment variables will be prepended with "STANDARDCI-". This name is a first proposal for the idea and it may get changed based on feedback. | ||
|
||
* **STANDARDCI\_REPOSITORYCOMMITID** -- Commit hash / ID; Example: 2ba93796dcf132de447886d4d634414ee8cb069d | ||
* **STANDARDCI\_REPOSITORYROOT** -- Root of repository; Example: D:\repos\corefx |
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.
this should have a Unix file path example as well.
|
||
## Support from CI Services | ||
|
||
This plan will only work if CI services decide to support these environment variables. An important question is whether CI services have similar environment variables already. The table below suggests that the information we need is already available. An arbitrary sample of CI services were picked for this exercise. |
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.
If the CI services don't add support for the new env vars could the .NET SDK translate these variables below to the STANDARDCI_
ones and re-export them?
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.
@akoeplinger In theory we could, but then we would have variables specific to some set of CI systems in the .NET SDK, which is undesirable.
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.
Ya. Doing that is kinda the opposite of what we want from this plan.
|
||
## Proposed environment variables | ||
|
||
The .NET Core SDK needs are oriented around source control. As a result, the intial list is source control oriented, but there is no affinity to source control on the general idea of standardized environment variables. |
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.
I think the specification for some the proposed environment variables is too vague. The consuming application probably relies on a specific format or otherwise needs to deal with missing/unexpected info.
E.g. can STANDARDCI_REPOSITORYCOMMITID
only be a git hash? If not, this needs to be treated as a free form string. Same for STANDARDCI_REPOSITORYNAME
unless we want to prescribe the org/repo format.
Can any of the variables be empty if no sensible information is available?
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.
@akoeplinger Good points. We should specify what we expect of the value.
There are a few uses of the commit id we are thinking of at this moment:
- Use it in source link to map local paths to URLs on source server.
This is the most important usage that also implies most restrictions on the string. @richlander we need to get in touch with various source control providers to see what URLs are they using for their content servers.
- Use it in
AssemblyInformationVersion
attribute like so:
<InformationalVersion>$(Version). Commit: $(STANDARDCI_REPOSITORYCOMMITID)</InformationalVersion>
This way one can identify the corresponding source commit easily just by looking at an assembly level attribute. This is just a display string with no specific format requirements.
- Include it in a description of a NuGet package again to allow associating particular package version with a source commit. This is also a display string with no specific format requirements.
* Add AWS CodeBuild environment variables * Add link to AWS CodeBuild environment variables documentation
cc @tmds I wonder how this combines with our env vars for s2i builds (https://github.com/redhat-developer/s2i-dotnetcore/tree/master/2.0/build#environment-variables) and runs (https://github.com/redhat-developer/s2i-dotnetcore/tree/master/2.0/runtime#environment-variables). |
@omajid great question. I see a disjoint set of environment variables. So, you could expand your set to include this information. Other thoughts? |
@richlander Definitely, we can add more where appropriate. I am also curious about the use cases that prompted this design doc (and whether some modified forms of vars from s2i should be added here if appropriate). |
@omajid Good question. Will share the spec soon. In absence of that, there are two main scenarios:
More Info: |
* **STANDARDCI\_REPOSITORYCOMMITID** -- Commit hash / ID; Example: 2ba93796dcf132de447886d4d634414ee8cb069d | ||
* **STANDARDCI\_REPOSITORYROOT** -- Root of repository; Example: D:\repos\corefx | ||
* **STANDARDCI\_REPOSITORYNAME** -- Name of repository; Example: dotnet\corefx | ||
* **STANDARDCI\_REPOSITORYURI** -- Uri for repository; Example: https://github.com/dotnet/corefx |
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.
What are the use-cases for _REPOSITORYROOT and _REPOSITORYNAME?
@omajid There is also an envvar (OPENSHIFT_BUILD_REFERENCE) that contains the ref (e.g. branch name), this is not used in this proposal. |
* **STANDARDCI\_REPOSITORYCOMMITID** -- Commit hash / ID; Example: 2ba93796dcf132de447886d4d634414ee8cb069d | ||
* **STANDARDCI\_REPOSITORYROOT** -- Root of repository; Example: D:\repos\corefx | ||
* **STANDARDCI\_REPOSITORYNAME** -- Name of repository; Example: dotnet\corefx | ||
* **STANDARDCI\_REPOSITORYURI** -- Uri for repository; Example: https://github.com/dotnet/corefx |
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.
What happens when project was built from multiple VCS repositories and commits? These variables are designed only for single VCS repository case.
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 think that if this is going to be an all-CI standard, then it must clearly state what happens to these variables if there is more then one repository involved in the CI build (not a rare thing at all in our world BTW).
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.
That's a great question. Two thoughts ...
- Do single binaries get built from multiple repos?
- How do the existing CI environment variables behave in that scenario?
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.
- Can't rule out for all cases. In our scenarios, often you might choose one primary repository which is mostly responsible for making the output binary. Anyways, there're no CI settings for this choosing, it goes untold.
- Considering TeamCity,
build.vcs.number
is only present if there is one source control attached to the build. Otherwise there would be multiple variables calledbuild.vcs.number.id-of-source-control-root-here
(each source control root which you attach to this build has an ID in TeamCity), one per root, with their respective revision numbers.teamcity.build.checkoutDir
isn't the repository root at all, actually. It's just the working root of the build run. When attaching a source control root, you specify the folder (or folders) where its files go. Even in case of one single root that does not have to be the working directory root. I'm not sure if you can get the actual repository root folders at all in TC props.
|STANDARDCI\_REPOSITORYCOMMITID | BUILD\_SOURCEVERSION | TRAVIS\_COMMIT |APPVEYOR\_REPO\_COMMIT | CIRCLE\_SHA1 | CODEBUILD_RESOLVED_SOURCE_VERSION | | ||
|STANDARDCI\_REPOSITORYROOT|BUILD\_REPOSITORY\_LOCALPATH|TRAVIS\_BUILD\_DIR| APPVEYOR\_BUILD\_FOLDER | CIRCLE\_WORKING\_DIRECTORY | CODEBUILD_SRC_DIR | | ||
|STANDARDCI\_REPOSITORYNAME|BUILD\_REPOSITORY\_NAME| TRAVIS\_REPO\_SLUG | APPVEYOR\_REPO\_NAME | CIRCLE\_PROJECT\_USERNAME + CIRCLE\_PROJECT\_REPONAME | | ||
|STANDARDCI\_REPOSITORYURI|BUILD\_REPOSITORY\_URI| | | CIRCLE\_REPOSITORY\_URL | CODEBUILD_SOURCE_REPO_URL | |
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.
In case of TeamCity you have the following build parameters which could be used during the build:
STANDARDCI_REPOSITORYCOMMITID
-> build.vcs.number
STANDARDCI_REPOSITORYROOT
-> teamcity.build.checkoutDir
STANDARDCI_REPOSITORYNAME
-> vcsroot.username
+ env.TEAMCITY_PROJECT_NAME
STANDARDCI_REPOSITORYURI
-> vcsroot.url
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.
Yet again this only works in case of a single source control root.
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.
Thanks @dtretyakov. Added to doc.
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.
@dtretyakov: what is the idea around vcsroot.username
+ env.TEAMCITY_PROJECT_NAME
? The first is an empty string in our builds, and the second is just the name of the parent entity in TeamCity hierarchy.
Good initiative! Agree to the other commenters that the use cases should probably be primary when deciding on the set and meaning of the variables, so they need to be described first. For the task of retrieving the sources, providing a pattern for a URL with a substitution for the relative file path might work best. |
The C# support on Travis CI relies on community-support (I see that one of the maintainers—@akoeplinger—has already been involved here). What this means in this case is that we will defer decisions to the maintainers. When it has been agreed on what the environment variables should be named, it would be very straightforward for our community maintainers to implement them into the build process. The repo URI is currently blank in the table for Travis CI, but it is not hard to get it. Thank you! |
Excellent feedback folks. I added a bunch more information into the doc that should provide more context. I hope that helps. After conversation today with @tmat, we realized that we only need two variables. We have a related plan that we are working on (will share soon) that won't work with environment variables since we want to support git submodules. |
Since Jenkins gets used a lot as an on-prem CI, it's worth mapping the variables too:
@richlander I wonder. Don't you need to know the 'REPOSITORYROOT' to strip that off the full path of the source files when creating pdbs? |
@tmds feel free to PR the Jenkins info in. I couldn't find a good page that documents those env variables. Maybe you can find one. You are right that some additional scenarios will need the repository root. We decided (again, spec coming soon) that we cannot use environment variables for those scenarios. It's only this one that works well with environment variables. |
Why does the plan not include reaching out to authors of other application environments? |
|
||
## Proposed Standard Environment Variables | ||
|
||
The .NET Core SDK needs are oriented around source control. As a result, the intial list is source control oriented, but there is no affinity to source control on the general idea of standardized environment variables. |
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.
intial
-> initial
.
@svick good suggestion. Any thoughts on which application platforms to start with and where to post such requests for feedback? BTW: If other people want to post such requests for feedback in other application platform forums, please go ahead. |
Hi guys, is the spec ready? |
@FeodorFitsner Yes, I think that me and @tmat are good with the spec now. I suspect that others may still have more feedback, however. |
@richlander Great, we'll go ahead with implementation then: appveyor/ci#1939 |
@richlander https://wiki.jenkins.io/display/JENKINS/Building+a+software+project Should we discuss the names themselves? I find them long and hard to remember (e.g. STANDARDCI_REPOSITORYCOMMITID). |
@tmds We propose These variables are not something that end users would type often and have to remember. Instead, they are meant to be used by SDKs and tools. |
Using these variables in the git-case does not limit the usage to git. If you want to use these variables to checkout the sources for different scms, how do you distinguish between the scms? e.g if the REPOSITORYURI is an https uri, how do you know if this is git, svn, ...? |
Maybe just |
@poke |
Yeah, I hesitated longer about whether I should leave out the But I understand if that sounds just like a lame attempt to come up with an explanation—I could live with a longer name that includes “repository” as well ;) |
@poke Isn't the ID tied to the main repository though? Sure, there may be submodules, but their commit ids are determined by the containing repo commit id. |
@tmat did you see my question (#19 (comment))? how do you distinguish between scms?
The former is subversion, the latter is git. |
@tmds We do not need to distinguish for the purposes described in the spec: https://github.com/dotnet/designs/pull/19/files#diff-c822e2b5a39c88f075f12b081589c031R18 |
@tmat I was more referring to how with DVCS, I can have many repositories for the same project (with different URIs) but a revision id (commit id) is a unique identifier that is globally valid for all repositories. So yes, a revision is created within a repository but then not limited to that repository. |
@poke I see what you mean. I'd say it can still be That said, perhaps for clarity a different name would be better. How about |
@tmat ah ok, I was envisioning more advanced use-cases where a tool had sufficient info to check out and map to the source files. The scenarios described involve a human to interpret the info. |
@tmds That could be an interesting use case if there is interest in it. I'd start simple though. We can always make things more complicated. The other way is harder :) |
@tmat Yeah, I guess you’re right. I worry though that with just these two variables, this essentially turns into a super long prefix in addition to the If we choose a different word, I would suggest And btw. I think we should agree on whether we want to use underscores between the words or not. I would hate having both |
@poke Agreed on underscores and consistency. Is I actually prefer long names if they add clarity. This is not something anyone would need to type all the time. |
@tmat Yup, those look good to me! (URL or URI though? xD) |
I prefer URL - as it comes more naturally to me when typing it, but i don't really care. |
Updated the proposal based on the above discussion. |
Moved to accepted specs via #32. |
The variables defined above are now used in dotnet/msbuild#3063 |
I have moved this proposal to "accepted": https://github.com/dotnet/designs/blob/master/accepted/build/standard-ci-env-variables.md |
/cc @tmat