Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Turn on Dependabot and CodeQL #139

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

cooljeanius
Copy link

These are just a few YAML files to help integrate a bit more tightly with some of the services that GitHub provides. I also wanted to try migrating the Travis workflow to GitHub Actions, but couldn't, because the GitHub Actions Importer is stupidly overengineered, and requires a running copy of Docker Desktop for some reason, and a recent update to Docker Desktop started using symbols only available on macOS 12 (and up) without adding any sort of compatibility note, and I'm still on macOS 11...

@cooljeanius
Copy link
Author

I see this as a prerequisite for taking #138 out of draft mode, btw

Copy link
Member

@neverpanic neverpanic left a comment

Choose a reason for hiding this comment

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

Can you also rebase those changes to remove the merge commits?
Alternatively, I guess we could squash them on merge.

.github/dependabot.yml Outdated Show resolved Hide resolved
@@ -0,0 +1,82 @@
# trac-github/.github/workflows/codeql.yml: GitHub Actions codeql workflow for trac-github
Copy link
Member

Choose a reason for hiding this comment

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

I'm not familiar with CodeQL. Can you elaborate what the added value of this for this repository is?

Copy link
Author

Choose a reason for hiding this comment

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

Right, so CodeQL is basically another linter / static analyzer / set of compiler warnings to help ensure future commits or PRs don't break anything. I'd feel a lot more secure about taking #138 out of draft mode if I could get a set of CI checks (such as CodeQL) set up first to verify that it'd be ok to merge.

Copy link
Member

Choose a reason for hiding this comment

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

Do you have more details than that? From what I see in the marketing material, this sounds like a closed-source third-party tool, which I cannot run locally (or only a limited version) unless I pay for a GitHub Enterprise something thingy.

Is there some way we could see what warnings CodeQL would find in this existing code base? I don't want an opinionated linter that throws a thousand warnings that nobody is ever going to fix in this code.

From a run in your fork, it seems this doesn't even work as expected, because this is still Python 2 code, but Python 2 isn't installed:

Python package installation failed: we detected this code as Python 2, but the 'python2' executable was not available. To enable automatic package installation, please install 'python2' before the 'github/codeql-action/init' step, for example by running 'sudo apt install python2' (Ubuntu 22.04). If your code is not Python 2, but actually Python 3, please file a bug report at https://github.com/github/codeql-action/issues/new

https://github.com/cooljeanius/trac-github/actions/runs/7018282343/job/19093341421

So it seems that in its current state, this CodeQL run doesn't actually do anything.

Copy link
Author

Choose a reason for hiding this comment

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

Do you have more details than that? From what I see in the marketing material, this sounds like a closed-source third-party tool, which I cannot run locally (or only a limited version) unless I pay for a GitHub Enterprise something thingy.

CodeQL sources are available here under an MIT license: https://github.com/github/codeql

Is there some way we could see what warnings CodeQL would find in this existing code base? I don't want an opinionated linter that throws a thousand warnings that nobody is ever going to fix in this code.

From a run in your fork, it seems this doesn't even work as expected, because this is still Python 2 code, but Python 2 isn't installed:

Python package installation failed: we detected this code as Python 2, but the 'python2' executable was not available. To enable automatic package installation, please install 'python2' before the 'github/codeql-action/init' step, for example by running 'sudo apt install python2' (Ubuntu 22.04). If your code is not Python 2, but actually Python 3, please file a bug report at https://github.com/github/codeql-action/issues/new

Ah ok, let me try that...

Copy link
Member

Choose a reason for hiding this comment

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

Quoting from https://github.com/github/codeql:

The CodeQL CLI (including the CodeQL engine) is hosted in a different repository and is licensed separately.

Copy link
Author

@cooljeanius cooljeanius Dec 6, 2023

Choose a reason for hiding this comment

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

OK, here's a new run:
https://github.com/cooljeanius/trac-github/actions/runs/7118192100/job/19380559866
It still doesn't produce any warnings, but that's good, it means that this codebase doesn't have anything worth warning about. The idea is to prevent that from changing in potential future PRs.
Here's an example of a place where CodeQL does actually warn in some Python code: https://github.com/cooljeanius/jhbuild/security/code-scanning/1

Copy link
Author

Choose a reason for hiding this comment

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

@neverpanic better now?

cooljeanius and others added 3 commits November 28, 2023 06:47
change weekly to monthly

Co-authored-by: Clemens Lang <[email protected]>
Install python2 before initializing CodeQL
Use github-actions ecosystem
bump action versions used:
- checkout: 3 to 4
- codeql-action: 2 to 3
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