Add requirements-test.txt to use on CI - #29
Conversation
| # Permission is hereby granted, free of charge, to any person obtaining a copy | ||
| # of this software and associated documentation files (the "Software"), to deal | ||
| # in the Software without restriction, including without limitation the rights | ||
| # to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
| # copies of the Software, and to permit persons to whom the Software is | ||
| # furnished to do so, subject to the following conditions: | ||
| # The above copyright notice and this permission notice shall be included in | ||
| # all copies or substantial portions of the Software. | ||
| # THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
| # IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
| # FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
| # AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
| # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
| # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | ||
| # SOFTWARE. |
There was a problem hiding this comment.
I don't think we need to but the entire license into the requirements file. What about adding the license identifer
| # Permission is hereby granted, free of charge, to any person obtaining a copy | |
| # of this software and associated documentation files (the "Software"), to deal | |
| # in the Software without restriction, including without limitation the rights | |
| # to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | |
| # copies of the Software, and to permit persons to whom the Software is | |
| # furnished to do so, subject to the following conditions: | |
| # The above copyright notice and this permission notice shall be included in | |
| # all copies or substantial portions of the Software. | |
| # THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | |
| # IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | |
| # FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | |
| # AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | |
| # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | |
| # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | |
| # SOFTWARE. | |
| # SPDX-License-Identifier: Apache-2.0 |
instead and defaulting to Apache-2.0 which seems the default for this repo and not adding another license.
There was a problem hiding this comment.
I don't have a strong preference, mostly following what I saw in other files, such as:
What would you like, @obersteiner ?
There was a problem hiding this comment.
Looking closer at the README, it says:
The original libcudacxx and LLVM's libc++ are distributed under the Apache License v2.0 with LLVM Exceptions. Any new files and modifications made to existing files by AMD are distributed under MIT.
So we should license under MIT. New guidelines say to add the copyright header + SPDX-license identifier only.
There was a problem hiding this comment.
Ok, trimmed to # SPDX-License-Identifier: MIT
## Motivation * Progress on ROCm/TheRock#7491 * See also ROCm/libhipcxx#29 * See also #11396 We've been using a common https://github.com/ROCm/TheRock/blob/main/requirements-test.txt file across all ROCm subproject tests. libhipcxx and hipthreads are currently the only subprojects using `lit` during tests, so we can move the requirement into project-local files that are only installed as part of testing those subprojects. ## Test Plan I'm testing this together with some changes in TheRock that will start picking up this requirements file from the test artifact. See https://github.com/ROCm/TheRock/actions/runs/33191445520/job/98940712134. It should be safe to add the requirements here and then start using them from TheRock later. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
Description
We've been using a common https://github.com/ROCm/TheRock/blob/main/requirements-test.txt file across all ROCm subproject tests and these packages are relatively large to download for all subprojects on runners with slow network (sccache is 8-15MB), so I want each project to declare which test requirements it has directly and leave only CI system and test runner requirements in the common file.
The full libhipcxx source is already included in the 'test' artifact in TheRock via https://github.com/ROCm/TheRock/blob/950330ecc2554996ff6a424780a81bf1aff64e32/math-libs/artifact-libhipcxx.toml#L10-L18, so this won't need extra work to start distributing.
Test Plan
I'm testing this together with some changes in TheRock that will start picking up this requirements file from the test artifact. See https://github.com/ROCm/TheRock/actions/runs/33191445520/job/98940713056. It should be safe to add the requirements here and then start using them from TheRock later.
Checklist