search

ZedThree / clang-tidy-review

Create a pull request review based on clang-tidy warnings
MIT License
88 stars 44 forks source link

Option "apt_packages" can't find packages #57

Closed marcluque closed 1 year ago

marcluque commented 1 year ago

We have started to use this action in our pipeline (see here for the YAML file). Unfortunately, it is not installing our desired packages, which can be seen here in an action log.

I was wondering why this is the case and I'm speculating that an apt update could be missing. As far as I can tell, the action is not doing that as of now. With regards to the installation of the packages I looked at these lines in review.py.

We'd appreciate any help, thank you in advance for your time!

FlorianReimold commented 1 year ago

I think your quick solution would be to remove the space in between your packages:

apt_packages: libsdl2-dev,libcriterion-dev

At least it works in our gh action that way.

It's still a reasonable assumption that it should work like that in my opinion, so maybe someone should add a strip() call before passing the list as arguments.

marcluque commented 1 year ago

Ouh interesting, very good catch, thank you!

Unfortunately, the result remains the same (see here)

marcluque commented 1 year ago

At least it works in our gh action that way.

I'm not sure if this is related, but maybe using Ubuntu 20.04 vs 22.04 makes a difference here. That's something I noticed when looking at your Dockerfile.

When, for example, asking for the ninja-build package, I'm getting this: image Full log

FlorianReimold commented 1 year ago

Hm, yea, we are still using the old Ubuntu 20 based docker image... The package is available on Ubuntu 22.04 though, I tried in a VM i have available locally. But that VM runs a Desktop version of Ubuntu 22.04.

marcluque commented 1 year ago

Indeed, I checked the package list too, thank you for trying the VM! I just created a fork and added apt-get update before the install command (https://github.com/marcluque/clang-tidy-review/commit/e75a77d8f1359e0537d98a2b1f03c541f21b4b23) and now it works 🎉

So a possible fix would actually be to update the package sources beforehand 🤔

ZedThree commented 1 year ago

Updating the packages seems like a good idea! Please could you make a PR @marcluque ?

marcluque commented 1 year ago

Updating the packages seems like a good idea! Please could you make a PR @marcluque ?

Glady! I've opened a PR.

ZedThree commented 1 year ago

Closed in #58