Problem with Jenkins CI due to "sys.stdin.isatty" check

[omarkohl]

When using gitlint on Jenkins it doesn't work as it does locally simply by executing "gitlint" to check the latest commit.

gitlint

Since on Jenkins there is not tty present the command simply fails with several cryptical warnings (probably because stdin is empty). When using the -d option it tells me DEBUG: gitlint.lint Linting commit [SHA UNKNOWN] which was even more confusing.

After reading the code and re-reading the documentation I ended up sucessfully using this:

git log -1 --pretty=%B | gitlint

This is a little cumbersome and it seems unecessary to base the decision whether to perform GitContext.from_local_repository depending on if there is a TTY present or not.

Suggestions:

• Always use from_local_repository if there is a local repository AND (stdin is empty OR not tty). Use stdin in other cases.
• Alternatively or additionally document this more clearly. Maybe a doc section on how to use gitlint with CI tools.

[jorisroovers]

So I think that depends on how you have your Jenkins configured. I've been doing it this way on Jenkins for years (on multiple different Jenkins installations) - so it can definitely work.

I do agree that we can probably be a bit smarter about TTY detection and make the CI section in our docs more prominent.

Marking this as a feature enhancement. I don't know when I'll get to this - it's likely to take a while (I don't have as much time to work on this anymore).

Thanks for your interest and submitting this issue!

[omarkohl]

Could you explain how you do it in Jenkins? Because I'm not aware of doing anything special. Just a regular Freestyle project with an "Execute shell" Build step.

Concerning the documentation I did read the part you linked to and it was actually where I got the final solution from. But it doesn't explain that this is actually necessary when not using a TTY. I only understood that when reading the Python code.

The resolution of this ticket has very low priority for me because now it works (as described above). But it might be helpful as documentation in case someone else runs into the issue.

[jorisroovers]

So I looked into this a bit more, the Jenkins's I've worked with always had jobs configured to run in containers with a TTY attached - which explains it working for me.

Anyways, I changed gitlint to be smarter on how it detects what to do, removing the TTY detection piece entirely (basically, it just checks whether anything is being passed to STDIN at startup, if not, it falls back to the local repo).

I think that solves this issue :) Let me know if not. I'm releasing this as part of 0.9.0 in the next few hours.

[omarkohl]

Thanks for doing this so quickly but unfortunately it doesn't work for me.

DEBUG: gitlint.cli Platform: Linux-3.10.0-693.5.2.el7.x86_64-x86_64-with-centos-7.4.1708-Core
DEBUG: gitlint.cli Python version: 3.4.5 (default, May 29 2017, 15:17:55) 
[GCC 4.8.5 20150623 (Red Hat 4.8.5-11)]
DEBUG: gitlint.cli Git version: git version 1.8.3.1
DEBUG: gitlint.cli Gitlint version: 0.9.0
DEBUG: gitlint.cli Configuration
...
DEBUG: gitlint.lint Linting commit [SHA UNKNOWN]
DEBUG: gitlint.lint Commit Object
Author: None <None>
Date:   None

Gitlint is installed with pip3 install --user -U gitlint and the Jenkins job executes the following:

~/.local/bin/gitlint -d

[jorisroovers]

Re-opening. Will look at this again (not sure when).

[pbregener]

The same problem exists when using GitLab CI.
While git log -1 --pretty=%B | gitlint can be a workaround for many cases, it obviously doesn't work if you want to enforce rules on author names or email addresses.

[pbregener]

My current workaround for this is to patch gitlint by importing below custom "rule":

import gitlint

def always_return_false():
  return False

gitlint.cli.stdin_has_data = always_return_false

[jorisroovers]

Ok, so I spend some more time on this. I was able to reproduce this issue in Jenkins and also was able to determine that the fix proposed in #52 by @bdrung unfortunately doesn't work in Jenkins.

I've made a new attempt in the https://github.com/jorisroovers/gitlint/tree/issues/42 branch based on ideas from #52. @pbregener, you kindly suggested to test a new fix in Gitlab CI in #56, I'd like to take you up on that offer :-)

Still need to do cleanup, tests, docs, etc - but would like to know if the fix works first.

Some more background for your interest and future reference:

The current attempted fix goes a step further than what was done in #52 by also checking whether the passed data isn't empty. This is neccessary, as in Jenkins, stat.S_ISFIFO(mode) returns True. I'm not a 100% sure, but my best guess is that Jenkins invoke the "Execute Shell" process with an empty piped input.

You can actually simulate this in bash like so:

echo "gitlint" > test-script.sh
$ echo -n "" | /bin/sh -xe test-script.sh

Even though the gitlint command in test-script.sh doesn't get any input piped into it, stat.S_ISFIFO(mode) will still return True because we're piping content into /bin/sh -xe test-script.sh. In other words, in bash/shell, pipes seem to trickle down to subprocesses (which actually makes sense).

To remedy this, I added a check on the length of sys.stdin.read() in case stat.S_ISFIFO(mode) or stat.S_ISREG(mode) is True.

I personally was avoiding doing this extra check in the past because I think it's a bit of a hack, but after spending more time on this, I don't think there's a way around in.

I think for the vast majority of users however, this will not be an issue at all. The only use-case that won't work is when you're trying to deliberatly lint an empty input string, like so:

echo -n "" | gitlint

In this case, the fix as suggested in issues/42 will fallback to reading from the local repository.

Note that it's actually pretty unlikely (but not impossible) to have this scenario occur in real world use-cases, as even an empty commit (git commit --allow-empty-message -m '') piped into gitlint like so git log --pretty=%B | gitlint will work as expected as git's pretty formatting returns a string with a line-ending (\n) which is obviously non-empty. As a result, gitlint will read from stdin and not the local repo.

I think this is an acceptable trade-off. We can always add extra --force-stdin and --force-local-repo parameters as suggested by #56 to work around this edge-case in the future.

Crossing fingers it works in Gitlab and other CI systems!

[pbregener]

I can confirm that it works in GitLab CI, too, @jorisroovers! 👍
Let me know if you need more details, I might have some time tomorrow..

[jorisroovers]

Great @pbregener, thanks! I'll see if I can wrap this patch up properly over the weekend.