Skip to content

Conversation

LeeTibbert
Copy link
Contributor

@LeeTibbert LeeTibbert commented Aug 18, 2019

  • Six months have passed and clang-9 rc-2 is available. Discover and
    use it if installed on the build system. This is the next
    in a long series of clang update PRs.

  • A year ago clang changed the format of its version suffix from containing a .0
    to not. For two iterations, both the -7 and -7.0 forms were looked up. This
    PR implements only a lookup on the 9 form. Enough transition time has passed.

  • Scala Native appears to build and run without notice using clang-9
    with and without lld-9. The only difference is the audible
    sizzle of Cool.

  • The Discovery code does not, and has not for years, discover
    llvm-config of the form llvm-config-N, where N is [3-9]. The current
    PR does no worse that prior art. A PR for another day.

Documentation:

  • The standard changelog entry is requested.

Testing:

  • Built and tested ("test-all") in debug mode, Java 8, using sbt 1.2.8 on
    X86_64 only . All tests pass.

    Since clang-9 is not installed on Travis, this shows that this PR
    did not break the existing discovery process.

  • To show that the code change was effective on a system with
    clang-9 installed, I manually ran an sbt "sandbox/run" and examined
    the output of an immediately subsequent "last" command.
    The log file showed clang-9 in use and, where appropriate, lld-9.

  * Six months have passed and clang-9 rc-2 is available. Discover and
    use it if installed on the build system.  This is the next
    in a long series of clang update PRs.

  * Scala Native appears to build and run without notice using clang-9
    with and without lld-9. The only difference is the audible
    sizzle of Cool.

  * The Discovery code does not, and has not for years, discover
    llvm-config of the form llvm-config-N, where N is [3-9]. The current
    PR does no worse that prior art.  A PR for another day.

Documentation:

    * The standard changelog entry is requested.

Testing:

  * Built and tested ("test-all") in debug mode, Java 8, using sbt 1.2.8 on
    X86_64 only . All tests pass.

    Since clang-9 is not installed on Travis, this shows that this PR
    did not break the existing discovery process.

  * To show that the code change was effective on a system with
    clang-9 installed, I manually ran an sbt "sandbox/run" and examined
    the output of an immediately subsequent "last" command.
    The log file showed clang-9 in use and, where appropriate, lld-9.
Copy link
Member

@ekrich ekrich left a comment

Choose a reason for hiding this comment

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

LGTM. I think we need this for the new version of Metals which seems to check using Discover.clang() or clangPP() and gives an error if using clang version 9.0.0.

LeeTibbert added a commit to LeeTibbert/scala-native-fork that referenced this pull request Feb 29, 2020
…ng-9, if present.

  * clang-10 and clang-9 are now available. Discover and use the highest
    installed clang on the build system.  This is the next
    in a long series of clang update PRs. It supersedes pending PR scala-native#1686.

  * Scala Native appears to build and run test-all without notice using
    either clang-10 or clang-9.  The only difference is the audible
    sizzle of Cool.

  * The Discovery code does not, and has not for years, discover
    llvm-config of the form llvm-config-N, where N is [3-9]. The current
    PR does no worse that prior art.  A PR for another day.

  * This PR adds two clang version to the current discovery mechanism.
    That mechanism is now checking quite a load of ancient clang versions.

    I believe that the discovery mechanism should be changed to a
    "specification" mechanism.  That is, require developers to
    specify any desired clang version other than the system default "clang".
    I did not make this change to minimize churn whilst ScalaNative is
    undergoing re-organization.

Documentation:

    * The standard changelog entry is requested.

Testing:

  * Testing needs to show first safety, then efficacy.

  * Safety, not breaking when neither clang-10 nor clang-9 is installed,
    will be shown by Travis CI.

  * To show that the code change was effective on a system with both
    clang-10 and clang-9 installed, I manually added
    'nativeCompileOptions ++= Seq("-v")'to the settings in build.sbt for
    the sandbox project and then running that project.

    The log file showed clang-10 in use.

    I then ran "test-all" using clang-10 and all tests passed.

  * I manually tested as above on the same system with only clang-9 (and
    clang itself) installed.  clang-9 was used and all tests passed.
@LeeTibbert
Copy link
Contributor Author

Superseded by PR #1736.

@LeeTibbert LeeTibbert closed this Feb 29, 2020
LeeTibbert added a commit to LeeTibbert/scala-native-fork that referenced this pull request May 15, 2020
…ng-9, if present.

  * clang-10 and clang-9 are now available. Discover and use the highest
    installed clang on the build system.  This is the next
    in a long series of clang update PRs. It supersedes pending PR scala-native#1686.

  * Scala Native appears to build and run test-all without notice using
    either clang-10 or clang-9.  The only difference is the audible
    sizzle of Cool.

  * The Discovery code does not, and has not for years, discover
    llvm-config of the form llvm-config-N, where N is [3-9]. The current
    PR does no worse that prior art.  A PR for another day.

  * This PR adds two clang version to the current discovery mechanism.
    That mechanism is now checking quite a load of ancient clang versions.

    I believe that the discovery mechanism should be changed to a
    "specification" mechanism.  That is, require developers to
    specify any desired clang version other than the system default "clang".
    I did not make this change to minimize churn whilst ScalaNative is
    undergoing re-organization.

Documentation:

    * The standard changelog entry is requested.

Testing:

  * Testing needs to show first safety, then efficacy.

  * Safety, not breaking when neither clang-10 nor clang-9 is installed,
    will be shown by Travis CI.

  * To show that the code change was effective on a system with both
    clang-10 and clang-9 installed, I manually added
    'nativeCompileOptions ++= Seq("-v")'to the settings in build.sbt for
    the sandbox project and then running that project.

    The log file showed clang-10 in use.

    I then ran "test-all" using clang-10 and all tests passed.

  * I manually tested as above on the same system with only clang-9 (and
    clang itself) installed.  clang-9 was used and all tests passed.
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