stevedlawrence commented on a change in pull request #42: URL: https://github.com/apache/daffodil-vscode/pull/42#discussion_r736572574
########## File path: .github/workflows/rat-check.yml ########## @@ -0,0 +1,107 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +--- +name: Rat Check + +on: + push: + branches-ignore: [ 'dependabot/**' ] + pull_request: + types: [opened, synchronize, reopened] + +jobs: + check: + name: Java ${{ matrix.java_version }}, Scala ${{ matrix.scala_version }}, ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + distribution: [ temurin ] + java_version: [ 11, 16 ] + scala_version: [ 2.12.13 ] + os: [ubuntu-20.04, windows-2019] + include: + - os: ubuntu-20.04 + shell: bash + env_cc: clang-10 + env_ar: llvm-ar-10 + - os: windows-2019 + shell: msys2 {0} + env_cc: clang + env_ar: llvm-ar + runs-on: ${{ matrix.os }} Review comment: I don't think we need any of this matrix stuff. If we are going to split different checks into different workflows, we should minimize the number of jobs in each workflow. Apache Rat doesn't need to be run on different operating systems or java/scala versions. I would just pick one. ########## File path: .github/workflows/rat-check.yml ########## @@ -0,0 +1,107 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +--- +name: Rat Check + +on: + push: + branches-ignore: [ 'dependabot/**' ] + pull_request: + types: [opened, synchronize, reopened] + +jobs: + check: + name: Java ${{ matrix.java_version }}, Scala ${{ matrix.scala_version }}, ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + distribution: [ temurin ] + java_version: [ 11, 16 ] + scala_version: [ 2.12.13 ] + os: [ubuntu-20.04, windows-2019] + include: + - os: ubuntu-20.04 + shell: bash + env_cc: clang-10 + env_ar: llvm-ar-10 + - os: windows-2019 + shell: msys2 {0} + env_cc: clang + env_ar: llvm-ar + runs-on: ${{ matrix.os }} + defaults: + run: + shell: ${{ matrix.shell }} + env: + SBT: sbt -J-Xms1024m -J-Xmx5120m -J-XX:ReservedCodeCacheSize=512m -J-XX:MaxMetaspaceSize=1024m ++${{ matrix.scala_version }} + + steps: + + ############################################################ + # Setup + ############################################################ + + - name: Install Dependencies (Linux) + if: runner.os == 'Linux' + run: sudo apt-get install -y libmxml-dev + + - name: Install Dependencies (Windows) + if: runner.os == 'Windows' + uses: msys2/setup-msys2@v2 + with: + install: clang diffutils make pkgconf + path-type: inherit + + - name: Check out mxml source (Windows) + if: runner.os == 'Windows' + uses: actions/[email protected] + with: + repository: michaelrsweet/mxml + ref: v3.2 + path: mxml + + - name: Install mxml library (Windows) + if: runner.os == 'Windows' + run: | + # Our runtime2 tests may break if mxml library is compiled with clang + export AR=ar CC=cc + cd mxml + ./configure --prefix=/usr --disable-shared --disable-threads + make install + # Workaround for sbt hanging problem + echo "COURSIER_CACHE=$temp" >> $GITHUB_ENV + echo "COURSIER_CONFIG_DIR=$temp" >> $GITHUB_ENV + + - name: Setup Java + uses: actions/[email protected] + with: + distribution: ${{ matrix.distribution }} + java-version: ${{ matrix.java_version }} + + - name: Check out Repository + uses: actions/[email protected] + with: + fetch-depth: 0 + + ############################################################ + # Build & Check + ############################################################ + + - name: Compile + run: $SBT compile Review comment: Don't need to compile anything to run rat. This can be removed. ########## File path: build/extension.webpack.config.js ########## @@ -1,7 +1,19 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ Review comment: This change needs to be reverted. We cannot change the license of code we did not write. Note that if Apache Rat cannot figure out that this is MIT license, we may need to modify ratLicneses/ratLicenseFamilies so that it can recognize this, similar to what Daffodil does for passera ########## File path: build.sbt ########## @@ -38,18 +40,29 @@ lazy val commonSettings = { ) } +lazy val ratSettings = Seq( + ratLicenses := Seq( + ("BSD2 ", Rat.BSD2_LICENSE_NAME, Rat.LICENSE_TEXT_PASSERA) + ), + ratLicenseFamilies := Seq( + Rat.BSD2_LICENSE_NAME + ), + ratExcludes := Rat.excludes, + ratFailBinaries := true, +) + lazy val commonPlugins = Seq(BuildInfoPlugin, JavaAppPackaging, UniversalPlugin) lazy val `daffodil-debugger` = project .in(file(".")) - .settings(commonSettings) + .settings(commonSettings, ratSettings) .settings(publish / skip := true) .dependsOn(core) .aggregate(core) lazy val core = project .in(file("server/core")) - .settings(commonSettings) + .settings(commonSettings, ratSettings) Review comment: The ratSettings are global, and so only should be applied on he root project, which is the daffodil-debugger project. This can be removed. ########## File path: project/Rat.scala ########## @@ -0,0 +1,67 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import sbt._ + +object Rat { + + lazy val excludes = Seq( + + // git files + file(".git"), + + // json files -- these ones do not support comments + file("snippets/dfdl.json"), + file("package.json"), + file(".prettierrc"), + + // generate verstion typescript file + file("src/version.ts"), Review comment: This file is mentioned in .gitignore, which Apache Rat should use and ignore those files. So this shouldn't be needed. ########## File path: .github/workflows/rat-check.yml ########## @@ -0,0 +1,107 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +--- +name: Rat Check + +on: + push: + branches-ignore: [ 'dependabot/**' ] + pull_request: + types: [opened, synchronize, reopened] + +jobs: + check: + name: Java ${{ matrix.java_version }}, Scala ${{ matrix.scala_version }}, ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + distribution: [ temurin ] + java_version: [ 11, 16 ] + scala_version: [ 2.12.13 ] + os: [ubuntu-20.04, windows-2019] + include: + - os: ubuntu-20.04 + shell: bash + env_cc: clang-10 + env_ar: llvm-ar-10 + - os: windows-2019 + shell: msys2 {0} + env_cc: clang + env_ar: llvm-ar + runs-on: ${{ matrix.os }} + defaults: + run: + shell: ${{ matrix.shell }} + env: + SBT: sbt -J-Xms1024m -J-Xmx5120m -J-XX:ReservedCodeCacheSize=512m -J-XX:MaxMetaspaceSize=1024m ++${{ matrix.scala_version }} + + steps: + + ############################################################ + # Setup + ############################################################ + + - name: Install Dependencies (Linux) + if: runner.os == 'Linux' + run: sudo apt-get install -y libmxml-dev + + - name: Install Dependencies (Windows) + if: runner.os == 'Windows' + uses: msys2/setup-msys2@v2 + with: + install: clang diffutils make pkgconf + path-type: inherit + + - name: Check out mxml source (Windows) + if: runner.os == 'Windows' + uses: actions/[email protected] + with: + repository: michaelrsweet/mxml + ref: v3.2 + path: mxml + + - name: Install mxml library (Windows) + if: runner.os == 'Windows' + run: | + # Our runtime2 tests may break if mxml library is compiled with clang + export AR=ar CC=cc + cd mxml + ./configure --prefix=/usr --disable-shared --disable-threads + make install + # Workaround for sbt hanging problem + echo "COURSIER_CACHE=$temp" >> $GITHUB_ENV + echo "COURSIER_CONFIG_DIR=$temp" >> $GITHUB_ENV + Review comment: I don't think any of the above setup steps are needed to run Rat. All we need is Java (since we're running sbt) and to checkout this repo. ########## File path: project/Rat.scala ########## @@ -0,0 +1,67 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import sbt._ + +object Rat { + + lazy val excludes = Seq( + + // git files + file(".git"), + + // json files -- these ones do not support comments + file("snippets/dfdl.json"), + file("package.json"), + file(".prettierrc"), + + // generate verstion typescript file + file("src/version.ts"), + + // ignore images - daffiodil.jpg, arrow.svg + file("images"), Review comment: I would prefer that we list specific files, rather than directories, even if all the files in the directory should be excluded. This gives more confidence that each individual file was examined and explicitly excluded from the rat check. Note that arrow.svg should not be excluded, since it has a license header in it that Apache Rat should be able to figure out. And if it can't, we should add a ratLicenses/ratLicenseFamilies entry so that it can. ########## File path: project/Rat.scala ########## @@ -0,0 +1,67 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import sbt._ + +object Rat { + + lazy val excludes = Seq( + + // git files + file(".git"), + + // json files -- these ones do not support comments + file("snippets/dfdl.json"), + file("package.json"), + file(".prettierrc"), + + // generate verstion typescript file + file("src/version.ts"), + + // ignore images - daffiodil.jpg, arrow.svg + file("images"), + ) + + lazy val BSD2_LICENSE_NAME = "BSD 2-Clause License" + + lazy val LICENSE_TEXT_PASSERA = Review comment: As mentioned, we can probably remove this BSD2_LICENSE_NAME and LICENSE_TEXT_PASSERA. Though we *may* need to add ones for the MIT license and the CC0 license if Rat can't figure those out by default. ########## File path: build.sbt ########## @@ -38,18 +40,29 @@ lazy val commonSettings = { ) } +lazy val ratSettings = Seq( + ratLicenses := Seq( + ("BSD2 ", Rat.BSD2_LICENSE_NAME, Rat.LICENSE_TEXT_PASSERA) Review comment: This ratLicenses thing is to specify license headers that Apache Rat cannot figure out. It can't figure out the passera license used in Daffodil, so we have to manually tell Apache Rat what that license is, and what the header looks like. But there is no passera header code in this repo, so this isn't necessary. I think this ratLicenses and ratLicenseFamilies settings can potentially be removed (see below comments though, we might need it for other files). ########## File path: src/adapter/activateDaffodilDebug.ts ########## @@ -1,7 +1,19 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ +/* Review comment: This change should be reverted along with all the other files in this directory. We cannot change their licenses. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
