You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@yetus.apache.org by GitBox <gi...@apache.org> on 2022/09/17 05:37:44 UTC
[GitHub] [yetus] GauthamBanasandra opened a new pull request, #289: YETUS-1200. Add ability to exclude files from hadolint
GauthamBanasandra opened a new pull request, #289:
URL: https://github.com/apache/yetus/pull/289
* This PR exposes an option
`--hadolint-ignore-files` in the
hadolint plugin.
* All the files matching this
pattern will be excluded from
hadolint.
--
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: gitbox-unsubscribe@yetus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yetus] aw-was-here commented on pull request #289: YETUS-1200. Add ability to exclude files from hadolint
Posted by GitBox <gi...@apache.org>.
aw-was-here commented on PR #289:
URL: https://github.com/apache/yetus/pull/289#issuecomment-1261472623
> @aw-was-here thanks for the suggestion, but Yetus still doesn't exclude the file.
>
> I tried the following -
> ## Excluding using `--excludes` option
>
> ```shell
> YETUS_ARGS+=("--excludes=dev-support/docker/Dockerfile_windows_10")
> ```
> - [ci-hadoop.apache.org/blue/organizations/jenkins/hadoop-multibranch/detail/PR-4936/7/pipeline](https://ci-hadoop.apache.org/blue/organizations/jenkins/hadoop-multibranch/detail/PR-4936/7/pipeline/)
This run doesn't show an excludes.txt file in the build artifacts at all.... which isn't surprising since the --excludes takes a filename of regexes. (Passing raw filenames on the command line has proven to be a long-term mistake. Command lines get even more complex and/or super long. Thus why a lot of the options that used to exist that took those forms have been slowly getting removed or replaced. Also a reason why this PR as written wouldn't get approval to get committed.)
> This doesn't exclude the file either. Here's the Hadoop Jenkins run - [ci-hadoop.apache.org/blue/organizations/jenkins/hadoop-multibranch/detail/PR-4936/8/pipeline](https://ci-hadoop.apache.org/blue/organizations/jenkins/hadoop-multibranch/detail/PR-4936/8/pipeline)
This run does have an [excluded.txt](https://ci-hadoop.apache.org/view/Hadoop/job/hadoop-multibranch/view/change-requests/job/PR-4936/8/artifact/out/excluded.txt) file in the build artifacts but I'm not sure why hadolint still ran against it. There was a bug that I believed was fixed when you added a file and an exclusion for that same file at the same time... maybe it isn't actually fixed. I'll need to dig into why Yetus didn't pull the file out of the change list. If that is the case, then the exclude should get merged into main prior to the PR as a workaround.
> > Looking at that run, it looks like the real problem is that the Dockerfile isn't telling hadolint that it isn't an POSIX-type shell using the hadolint shell pragma.
>
> The `hadolint shell=powershell` pragma isn't supported on [hadolint v1.11.1](https://github.com/hadolint/hadolint/tree/v1.11.1?rgh-link-date=2022-09-28T16%3A39%3A46Z) which is what the current Hadoop CI uses.
Oof. That is super old and has quite a few bugs in it.
> I faced a lot of issues in upgrading hadolint and had to abandon that approach. Thus, I've resorted to excluding the file in the hadolint plugin itself.
The problem I see is that I don't really see a reason to take on Hadoop's technical debt. Hadoop is going to have to upgrade its other dependencies at some point.
--
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: gitbox-unsubscribe@yetus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yetus] GauthamBanasandra commented on pull request #289: YETUS-1200. Add ability to exclude files from hadolint
Posted by GitBox <gi...@apache.org>.
GauthamBanasandra commented on PR #289:
URL: https://github.com/apache/yetus/pull/289#issuecomment-1259808139
@aw-was-here I tried the following -
```bash
YETUS_ARGS+=("--excludes=Dockerfile_windows_10")
```
It seems like Yetus didn't exclude the file `Dockerfile_windows_10`. Please refer to this run for more details - https://ci-hadoop.apache.org/blue/organizations/jenkins/hadoop-multibranch/detail/PR-4936/3/pipeline/.
If the above file was excluded, then hadolint wouldn't have failed. Thus, I've added an explicit option to ignore files for the hadolint plugin.
--
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: gitbox-unsubscribe@yetus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yetus] GauthamBanasandra commented on pull request #289: YETUS-1200. Add ability to exclude files from hadolint
Posted by GitBox <gi...@apache.org>.
GauthamBanasandra commented on PR #289:
URL: https://github.com/apache/yetus/pull/289#issuecomment-1261170423
@aw-was-here thanks for the suggestion, but Yetus still doesn't exclude the file.
I tried the following -
## Excluding using `--excludes` option
```bash
YETUS_ARGS+=("--excludes=dev-support/docker/Dockerfile_windows_10")
```
Please note that I've used the path relative to its location in the Hadoop repo. Yetus is unable to exclude the file. Here's the Hadoop Jenkins run - https://ci-hadoop.apache.org/blue/organizations/jenkins/hadoop-multibranch/detail/PR-4936/7/pipeline/
## Excluding using `.yetus/excludes.txt`
This doesn't exclude the file either. Here's the Hadoop Jenkins run - https://ci-hadoop.apache.org/blue/organizations/jenkins/hadoop-multibranch/detail/PR-4936/8/pipeline
<hr>
> Looking at that run, it looks like the real problem is that the Dockerfile isn't telling hadolint that it isn't an POSIX-type shell using the hadolint shell pragma.
The `hadolint shell=powershell` pragma isn't supported on [hadolint v1.11.1](https://github.com/hadolint/hadolint/tree/v1.11.1) which is what the current Hadoop CI uses. I faced a lot of issues in upgrading hadolint and had to abandon that approach. Thus, I've resorted to excluding the file in the hadolint plugin itself.
--
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: gitbox-unsubscribe@yetus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yetus] GauthamBanasandra commented on pull request #289: YETUS-1200. Add ability to exclude files from hadolint
Posted by GitBox <gi...@apache.org>.
GauthamBanasandra commented on PR #289:
URL: https://github.com/apache/yetus/pull/289#issuecomment-1276530656
Thanks a lot for the help @aw-was-here. I can confirm that the workaround is effective. Closing this.
--
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: gitbox-unsubscribe@yetus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yetus] aw-was-here commented on pull request #289: YETUS-1200. Add ability to exclude files from hadolint
Posted by GitBox <gi...@apache.org>.
aw-was-here commented on PR #289:
URL: https://github.com/apache/yetus/pull/289#issuecomment-1263813381
> I'll need to dig into why Yetus didn't pull the file out of the change list.
Inside start-dev-env.sh, a few observations:
* Running `dev-support/bin/test-patch --plugins=hadolint,maven --empty-patch` in the branch does not trigger any hadolint warnings.
* Running `dev-support/bin/test-patch --plugins=hadolint,maven,github GH:4936` fails.
* Removing the .yetus/excludes.txt and then running `dev-support/bin/test-patch --plugins=hadolint,maven --empty-patch --dirty-workspace` also fails.
So looks like the bug with excluding a file in the same PR is still present. But it might just be for hadolint since it keeps a separate file list rather than using the master one (which is probably a bug in and of itself).
Also of interesting, throwing:
```
/home/aw/hadoop/patchprocess/apache-yetus-0.14.0/lib/precommit/plugins.d/maven.sh: line 687: /tmp/yetus-24645.26567/maven-branch-dirlist-.txt: No such file or directory
```
error. Probably not related but interesting nonetheless.
--
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: gitbox-unsubscribe@yetus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yetus] aw-was-here commented on pull request #289: YETUS-1200. Add ability to exclude files from hadolint
Posted by GitBox <gi...@apache.org>.
aw-was-here commented on PR #289:
URL: https://github.com/apache/yetus/pull/289#issuecomment-1269085576
OK, I've tracked down why hadolint exclusions aren't working. I'll file a new JIRA to fix that.
--
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: gitbox-unsubscribe@yetus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yetus] aw-was-here commented on pull request #289: YETUS-1200. Add ability to exclude files from hadolint
Posted by GitBox <gi...@apache.org>.
aw-was-here commented on PR #289:
URL: https://github.com/apache/yetus/pull/289#issuecomment-1250130278
Is there a reason the generic [excludes](https://yetus.apache.org/documentation/0.14.0/precommit/usage-intro/#excluding-files) aren't good enough?
--
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: gitbox-unsubscribe@yetus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yetus] aw-was-here commented on pull request #289: YETUS-1200. Add ability to exclude files from hadolint
Posted by GitBox <gi...@apache.org>.
aw-was-here commented on PR #289:
URL: https://github.com/apache/yetus/pull/289#issuecomment-1270415343
YETUS-1202
--
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: gitbox-unsubscribe@yetus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yetus] aw-was-here commented on pull request #289: YETUS-1200. Add ability to exclude files from hadolint
Posted by GitBox <gi...@apache.org>.
aw-was-here commented on PR #289:
URL: https://github.com/apache/yetus/pull/289#issuecomment-1259912990
Looking at that run, it looks like the real problem is that the Dockerfile isn't telling hadolint that it isn't an POSIX-type shell using the hadolint shell pragma.
But as far as exclusions go, it be the path+filename, not just a filename. It is also generally easier to use [.yetus/excludes.txt](https://github.com/apache/yetus/blob/main/.yetus/excludes.txt) rather than depend upon a command line option so that people running it from the CLI will also pick up the exclusion.
--
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: gitbox-unsubscribe@yetus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yetus] GauthamBanasandra commented on pull request #289: YETUS-1200. Add ability to exclude files from hadolint
Posted by GitBox <gi...@apache.org>.
GauthamBanasandra commented on PR #289:
URL: https://github.com/apache/yetus/pull/289#issuecomment-1269264065
Thanks @aw-was-here . I'll try to use the workaround by first committing the .yetus/excludes.txt and then adding the actual file in a separate commit in the meantime. Could you please share the new JIRA here?
--
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: gitbox-unsubscribe@yetus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yetus] GauthamBanasandra closed pull request #289: YETUS-1200. Add ability to exclude files from hadolint
Posted by GitBox <gi...@apache.org>.
GauthamBanasandra closed pull request #289: YETUS-1200. Add ability to exclude files from hadolint
URL: https://github.com/apache/yetus/pull/289
--
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: gitbox-unsubscribe@yetus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org