Skip to content
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

Nightly proccess preparations: Setting Enso version through the environment #3241

Merged
merged 27 commits into from
Feb 7, 2022

Conversation

mwu-tow
Copy link
Contributor

@mwu-tow mwu-tow commented Feb 1, 2022

Pull Request Description

This PR introduces new version-deciding logic to the Engine. The goal is twofold:

  1. The Engine version generated by SBT builds should be overridable by the caller (e.g. CI). This allows us building different versions (e.g. dev build, nightly build or official release) from the commit, with no changes to the sources. This is achieved by introducing ENSO_VERSION and ENSO_EDITION environment variables.
  2. Rather than keeping current release number hardcoded in many places in the repository, we replace it with a single, placeholder version named 0.0.0-dev. It will be used when ENSO_VERSION is not defined. Typical use case is a local, development build.

This 0.0.0 version needed a tweak in our tests to properly work. Many thanks to @radeusgd for all the help!

Also, CHANGELOG.md was moved to the repository root.

All these changes are needed for nightly builds.

Important Notes

Eventually in the future even those 0.0.0-dev directories should be gone, but this is a matter for separate task (restructuring the engine layout in the repository). Existing use-cases can likely be covered with some symlink trickery.

Checklist

Please include the following checklist in your PR:

  • The documentation has been updated if necessary.
  • All code conforms to the Scala, Java, and Rust style guides.
  • All documentation and configuration conforms to the markdown and YAML style guides.
  • All code has been tested where possible.

@mwu-tow mwu-tow requested a review from 4e6 as a code owner February 1, 2022 02:38
@mwu-tow mwu-tow marked this pull request as draft February 1, 2022 02:39
@mwu-tow mwu-tow force-pushed the wip/mwu/engine-version-from-env branch from bacc4db to f58054c Compare February 2, 2022 07:56
@mwu-tow mwu-tow self-assigned this Feb 2, 2022
@mwu-tow mwu-tow marked this pull request as ready for review February 2, 2022 08:04
@mwu-tow mwu-tow changed the title [WIP] Setting Enso version through the environment Nightly proccess preparations: Setting Enso version through the environment Feb 2, 2022
@mwu-tow mwu-tow mentioned this pull request Feb 2, 2022
4 tasks
Copy link
Contributor

@farmaazon farmaazon left a comment

Choose a reason for hiding this comment

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

Please update CODEOWNERS - I propose to remove the ownership of CHANGELOG entirely (as proposed in #3247 (review))

// EDIT: after a call we decided, that it is not needed.

Copy link
Contributor

@4e6 4e6 left a comment

Choose a reason for hiding this comment

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

LGTM, although I'm not sure how the library management will work when the edition got linked to the enso version. I'd like to see @radeusgd review here

val currentEdition = sys.env.getOrElse(
"ENSO_EDITION",
defaultDevEnsoVersion
) // Note [Default Editions]
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose, the Default Editions note should be also updated

scalacVersion: String,
graalVersion: String,
currentEdition: String,
stdLibVersion: String
): Seq[File] = {
val gitInfo = getGitInformation(log).getOrElse(fallbackGitInformation)
val isRelease = isReleaseMode
val isInternalDevelopmentBuild: Boolean =
ensoVersion == defaultDevEnsoVersion
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, we already have isRelease. Why not specify dev build as an inversion of release, like !isRelease?

Copy link
Member

Choose a reason for hiding this comment

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

We may want a development build not in the 'internal' mode - for example to test how it behaves with a changed version string, but not in release mode.
If I understand correctly, our usual CI will actually run with isInternalDevelopmentBuild == false as it will override the version (and so for example the CI artifacts will be not internal-dev but not release either).

Copy link
Member

Choose a reason for hiding this comment

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

This flag was actually used to instrument some badly behaving tests. I figured a different and better way to tackle this, so I think it can actually be removed.

@mwu-tow mwu-tow merged commit 4baad5f into develop Feb 7, 2022
@mwu-tow mwu-tow deleted the wip/mwu/engine-version-from-env branch February 7, 2022 14:14
@4e6 4e6 mentioned this pull request Feb 10, 2022
4 tasks
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.

4 participants