Page MenuHomeFreeBSD

[WIP] Port Submission for igt-gpu-tools
ClosedPublic

Authored by jfree on Aug 16 2022, 2:01 PM.

Details

Summary

Intel’s igt-gpu-tools serves as a generic testing suite for drm drivers. The igt suite is separated into tests and tools that target kms, memory management, and command submission. These tests are especially helpful for low-level reporting, transparent tracking of kernel changes, and efficient debugging of modern drm drivers.

Diff Detail

Repository
R11 FreeBSD ports repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

jfree requested review of this revision.Aug 16 2022, 2:01 PM
jfree created this revision.

Are all these patches upstreamed? Are you sure plist doesn't break depending on options selected?

graphics/igt-gpu-tools/Makefile
23

We usually try to keep these on as few lines as possible for consistency between ports.
python:3.0+ doesn't make much sense, you probably want just python ( https://cgit.freebsd.org/ports/tree/Mk/Uses/python.mk#n23 )

33

Just use SHEBANG_FILES= scripts/*.sh scripts/*.py

36

We usually try to keep these on as few lines as possible for consistency between ports.

38
40
43

We usually try to keep these on as few lines as possible for consistency between ports.
No need to set DOCS , https://docs.freebsd.org/en/books/porters-handbook/book/#makefile-options-default

51

Why not use the default desc?

lwhsu added inline comments.
graphics/igt-gpu-tools/Makefile
6

Not a big issue but we usually use case sensitive FreeBSD in formal place.

  • Updated pkg-plist to support toggling options
  • Adjusted Makefile thanks to advice from diizzy@ and lwhsu@
  • Updated pkg-plist to add missing files under TEST option
  • Added patch to POSIX-ify igt bash scripts
  • Updated core library dependencies
  • Added patch to POSIX-ify igt bash scripts

POSIX-ify the scripts is a nicer solution, but for posterity, adding BUILD_DEPENDS=bash:shells/bash fixed the build (on 14amd64) for your last revision.

I think that's fine but I'm more concerned of the overall state of the port with the amount of patches and no response regarding upstreaming. This is borderline unmaintainable and I doubt anyone will pick this up once committed and try to "sync" with upstream. Please try upstreaming as much as possible and add references to either PRs/merge requests and/or commits.

I think that's fine but I'm more concerned of the overall state of the port with the amount of patches and no response regarding upstreaming. This is borderline unmaintainable and I doubt anyone will pick this up once committed and try to "sync" with upstream. Please try upstreaming as much as possible and add references to either PRs/merge requests and/or commits.

I actively update the patches in a 'freebsd' branch of igt-gpu-tools based on upstream in a drm-kmod fashion. You can see the source here:
https://gitlab.freedesktop.org/jakesfreeland/igt-gpu-tools

I plan on maintaining the port until I can get the FreeBSD compatibility patches upstreamed. Problem being, a lot of these patches are inadequate and hacky. I don't want to open a merge request until I have proper solutions for FreeBSD.

graphics/igt-gpu-tools/Makefile
33

Unfortunately, there are shell scripts in scripts/ that do not include the proper file extension.
Although lazy, SHEBANG_FILES= scripts/* was the easiest way to do this.
This shouldn't be necessary in the future when/if my patch to POSIX-ify the scripts gets committed upstream.

43

I adjusted the OPTIONS_DEFINE and OPTIONS_DEFAULT to be on one line as suggested.
Although it is unnecessary to include DOCS in defaults, I like the idea that someone with minimal
ports experience can look at the Makefile and know what is installed by default. This eliminates the
need to delve into the Porter's Handbook and check what the ports infrastructure deems "default".
If this is not a matter of personal preference, I will change it right away.

Thanks for clarifying, great work! =)

Consider running portfmt -D Makefile.

graphics/igt-gpu-tools/Makefile
12

USES=gnome
USE_GNOME=cario

15

USE_GNOME=glib20

so overall

USES=...gnome...
USE_GNOME= cairo glib20
17
USES=...xorg
USE_XORG=pixman
32–33

Would it be better to unconditionally install the man pages rather than have an option? It's just one small build dependency.

Jake, I'm moving our email discussion here so others can weigh in if they have anything helpful to add. It looks like the build issues on 12.3 and 13.0 are related to changes in 160b4b922b6021848b6b48afc894d16b879b7af2 (and subsequent commits?) not available in those releases. If you are going to wait until September 1 to have this committed you don't have to worry about 13.0, however, 12.3 is still supported until three months after 12.4 is released. You may be able to patch your code by looking at /usr/include/sched.h. If it's not straightforward to work around the issue, you can add BROKEN_FreeBSD_12=sched.h definitions missing in 12.3. You might be able to come up with a better message.

In D36213#823470, @jrm wrote:

Consider running portfmt -D Makefile.

Joe,

Did portfmt generate these suggestions or are they from you?

USES=gnome
USE_GNOME= cairo glib20

USES=xorg
USE_XORG=pixman

I only get indentation suggestions when executing portfmt -D Makefile.

I will get back to you soon on whether the missing <sched.h> functions can be easily be reimplemented.

In D36213#823470, @jrm wrote:

Consider running portfmt -D Makefile.

Joe,

Did portfmt generate these suggestions or are they from you?

USES=gnome
USE_GNOME= cairo glib20

USES=xorg
USE_XORG=pixman

They are from me. A reason to prefer USES/USE_ is that changes can be made in one place to affect many ports.

I only get indentation suggestions when executing portfmt -D Makefile.

Indeed. I think it's valuable to have the consistent formatting from portfmt from the start, but others don't necessarily share this point of view. Your call.

I will get back to you soon on whether the missing <sched.h> functions can be easily be reimplemented.

I wouldn't spend too much time. I just mention it in case this is something that you've come across already with your igt-gpu-tools porting work and you may be able to come up with an easy fix.

@jrm
Nice catching the rest of USE and USES cases. I would also apprecate if these could be utilized more often as it makes maintaining the tree overall easier and less time consuming.

@jfree
I wouldn't spend too much time on 12.3 and 13.0, neither is unlikely to get fixes if we find issues so IMHO it makes more sense to set broken on those two versions and move on which is pretty much what @jrm suggests.

@jrm and @diizzy:

I took a quick glance at the <sched.h> file and decided to just mark 12.3 as broken.
I think this port will be stuck on CURRENT for a while anyway. I plan on contributing
some new src patches to fix some of the hacky ifdefs that get igt compiling on FreeBSD.
It is unlikely that my new src patches will be backported to FreeBSD 12 anytime soon.

  • Added USES for gnome and xorg per @jrm's advice
  • Marked FreeBSD 12 broken
  • Standardized formatting per portfmt
graphics/igt-gpu-tools/Makefile
68

Re-reading 13.13 of the Porter's Handbook, this should be IGNORE_FreeBSD_12, go after the License variables, and be set to something like

IGNRORE_FreeBSD_12= is unsupported on FreeBSD versions < 13.1.

  • Updated pkg-plist to support building on aarch64
  • Removed MANPAGES and VALGRIND options
  • Changed BROKEN_FreeBSD_12 to IGNORE_FreeBSD_12
  • Small formatting tweaks

Jake, are you satisfied this is ready to be committed? If so, I'll give it another look and commit if all looks good on my end.

In D36213#828576, @jrm wrote:

Jake, are you satisfied this is ready to be committed? If so, I'll give it another look and commit if all looks good on my end.

Hey Joe, I am speaking with Petri Latvala over at igt-gpu-tools to get a good chunk of the patches from this diff included in the igt-gpu-tools base.
I think it will be around 1-2 weeks before I get approval on their side. With that in mind, I think it is safe to commit this for now. I will open another
review at a later date to update this port to reflect the upstream changes.

graphics/igt-gpu-tools/Makefile
14

I think we need BUILD_DEPENDS= rst2man:textproc/py-docutils@${PY_FLAVOR}. I'll make that change when I commit, if you agree.

The only other minor changes I plan to make before committing:

  • Add entry in graphics/Makefile
  • Remove blank line between BUILD_DEPENDS and LIB_DEPENDS to quiet portlint.

@jrm Looks good. With those changes applied, the port should be ready to go. Thanks for checking it over again.

This revision was not accepted when it landed; it landed in state Needs Review.Sep 9 2022, 3:08 PM
This revision was automatically updated to reflect the committed changes.