Package evdi as akmod #97

ffgiff · 2019-06-25T20:22:29Z

Create spec files for evdi driver and displaylink userland module.
Add install test on Travis showing that Centos build fails (#13).
Fixes #81

ssaavedra · 2019-06-27T11:38:06Z

Thank you so much for your contribution.

First of all, the Travis test is something that is very interesting for supporting CentOS in the future, so it would be great to have that integrated, even if that is not the core value porposal of this PR.

On the akmod side of things, I'd rather have @kwizart review this here, if possible. I am not an expert on building akmods, and I don't know if there are any idioms or tests that should pass :-)

If @kwizart or the RPMFusion guys require a preliminary version to try this, I could trigger a Travis build for a pre-release branch from this uncommitted PR. Would that be useful?

kwizart · 2019-06-27T12:17:21Z

Our review process is very different from all travis/ci , so many files wouldn't be relevant.

Basically best would be to review the spec/src.rpm from bugzilla.rpmfusion.org.
Then, it will be possible to consider to merge back the spec here.

Our review process expect a dedicated bugzilla entry to be created with a spec and src.rpm publication.
Also you need to verify that the packages are building in mock (so there is no dependencies missing others than the one that are considered as a default in the buildroot).

Thx

kwizart

This is a quick review before you can submit each files into the RPM fusion review process.

Please do your best to fix the file and don't hesitate to ask question if anything.
Also please consider to use the bugzilla review process so others rpmfusion contributor can test/review.

kwizart · 2019-06-27T12:31:32Z

evdi-kmod-common.spec

@@ -0,0 +1,94 @@
+Name:          evdi-kmod-common


Please name this file evdi.spec
It will need to have :
Provides: evid-kmod-common = %{version}-%{release}

kwizart · 2019-06-27T12:36:25Z

evdi-kmod-common.spec

+Source4:	DisplayLink USB Graphics Software for Ubuntu %{_daemon_version}.zip
+Source5:        20-displaylink.conf
+License:       GPLv2.1, MIT and Proprietary
+ExclusiveArch:  i386 x86_64


You have used i686 and x86_64 in evdi-kmod, so you will have to use the same values on both side:
Please use i686 x86_64.

kwizart · 2019-06-27T12:38:57Z

evdi-kmod-common.spec

+Source2:        99-displaylink.rules
+Source3:        displaylink-sleep-extractor.sh
+# From http://www.displaylink.com/downloads/ubuntu.php
+Source4:	DisplayLink USB Graphics Software for Ubuntu %{_daemon_version}.zip


If you provide displaylink.spec, which seems appropriate given the few files that are maintained in the evdi source tree (scripts/library/etc), you will have to use this file only in the displaylink.spec

You mean so there should be three spec files: evdi, evdi-kmod and displaylink? That would work, but I'd need to move the DKMS part of displaylink.spec into a submodule to avoid the obvious clash.

RPM Fusion do not support dkms method, but you can support it if you like to (it would be a separate spec file).
We don't rely on your SCM at all if the spec are imported into RPM Fusion, it will have it's separate life.
You can have all spec into a "rpm" sub-directory of your project.

kwizart · 2019-06-27T12:39:21Z

evdi-kmod-common.spec

+# From http://www.displaylink.com/downloads/ubuntu.php
+Source4:	DisplayLink USB Graphics Software for Ubuntu %{_daemon_version}.zip
+Source5:        20-displaylink.conf
+License:       GPLv2.1, MIT and Proprietary


Please update the license as appropriate for earch components)

Yes, it's also wrong, it should be LGPLv2.1 for libevdi, with the MIT and proprietary bits being the bits in displaylink.spec

kwizart · 2019-06-27T12:40:16Z

evdi-kmod-common.spec

+BuildRequires:  make
+
+# No sources, so no debug package possible.
+%global debug_package %{nil}


Then It's probably noarch (specially once you will separate it from displaylink content).

kwizart · 2019-06-27T12:55:25Z

evdi-kmod-common.spec

+/etc/udev/rules.d/99-displaylink.rules
+/etc/X11/xorg.conf.d/20-displaylink.conf
+%dir /usr/libexec/displaylink
+/usr/libexec/displaylink/*


Please list files if possible instead of using wildcard.

I think in this case, if it's possible, it would be nice to have an exception to leave it that way so that we have less problems with forward-compatibility in case displaylink renames or adds new files here, and we miss those on our packaging update (which we might not notice in a CI environment without manual verification).

My opinion here is that hardening against delivering an incomplete release is likely a better trade-off than the alternative, but please feel free to convince me otherwise :-)

I understand that from a single package point of view, using wildcard seems convenient. But as a package maintainer, we want to ensure that no unexpected binaries will conflict with another package on end-user "before" they installed on their system.
This can be achieved by listing them explicitly.
So something like displaylink* is appropriate, but display* (it may conflicts with ImageMagick package that has a display binary among others).

In this case we are already inside /usr/libexec/displaylink, that's why I thought it would be safe enough. The case I can think of is that the user manually installed displaylink before installing the RPM, but that is an expected clash. Other packages should not be expected to need the /usr/libexec/displaylink folder, or should we not make that assumption?

Using libexec/displaylink is a good idea, specially for content that isn't supposed to be used by end-users directly. Should we expect some binaries to be used by end-users ?
Is this directory relevant for all binaries ?

kwizart · 2019-06-27T12:56:08Z

evdi-kmod-common.spec

+/etc/X11/xorg.conf.d/20-displaylink.conf
+%dir /usr/libexec/displaylink
+/usr/libexec/displaylink/*
+%dir /var/log/displaylink/


which daemon is resposible to output any log ? Are the log written as root ?

I assume the logs are written by the DisplayLinkManager process which is run by systemd. The logs are written as root.

kwizart · 2019-06-27T12:58:37Z

evdi-kmod.spec

+for kernel_version in %{?kernel_versions}; do
+  pushd _kmod_build_${kernel_version%%___*}/
+    make V=1 %{?_smp_mflags} \
+        KERNEL_UNAME="${kernel_version%%___*}" SYSSRC="${kernel_version##*___}" \


This was taken from the nvidia-kmod which is not the best case to use.
Please have a look on the variables that are appropriate for your project or use the kernel ones.

A quick test here is that you should be able to generate a valid evdi.ko for another kernel than the one you are currenly running on fedora (where a strict dependency is made for external modules to a given kernel uname -r)

kwizart · 2019-06-27T12:59:58Z

evdi-kmod.spec

+
+
+%changelog
+* Thu May 30 2019 ffgiff <ffgiff@gmail.com>


Best is to use the real name when possible. Also worth to mention the version , release at the end.
rpmdev-bumpspec is the tool to update that as appropriate.

kwizart · 2019-06-27T13:00:11Z

evdi-kmod.spec

+
+%changelog
+* Thu May 30 2019 ffgiff <ffgiff@gmail.com>
+        - First version


Ident, (use space)

ssaavedra · 2019-06-28T09:49:01Z

Thank you so much for this review and for the interest in contributing this to RPMFusion!

ffgiff · 2019-06-30T20:26:24Z

Thanks for the review, I'll update the PR in a week or so once I can get around to addressing all the issues. I think it does deserve some changes to be made in displaylink.spec, at least the dkms and libevdi parts need to be submodules so they won't clash with the akmod. That should make the licensing clearer too, but initially I was aiming to keep the change as unobtrusive as possible.

I'll also see about making a separate PR for the travis change.

The patches were provided by abucodonosor and severach. See DisplayLink/evdi#172 (comment) See DisplayLink/evdi#172 (comment)

….4-patch Apply patches to get evdi to compile and install on kernel 5.4.

…support for kernels 5.4 and 5.5.

When uninstalling, we need to remove the symbolic link first.

The changes are: * Updates related to the new evdi driver version 1.7.0. * Update to the released Displaylink driver 5.3.1. * The minimum kernel supported in evdi 1.7.0 is now 4.15. Adjusted the spec accordingly. * Remove Travis build for Fedora 27 since the kernel is no longer supported. * Fix support for DL-6xxx devices. The firmware image was not being copied from the DisplayLink driver package. * Adjust how we use dkms inside the rpm to follow recommended way outlined in the documentation. Resolves displaylink-rpm#112, resolves displaylink-rpm#121, resolves displaylink-rpm#128

Fixes related to evdi devel branch changes

Refactor spec file in preparation for new release The goal of these changes is to provide a more consistent experience when installing and upgrading. These changes have been tested over the past few weeks. Upgrading to new versions of the produced rpm has been tested. Upgrading to newer kernel versions has been tested. This commit does the following: * Update to evdi driver version 1.7.0. * Update to Displaylink driver 5.3.1. * The minimum kernel supported in evdi is now 4.15. Adjusting spec to match. * Fix support for DL-6xxx devices. The firmware image was not being copied from the DisplayLink driver package. * Adjust how we use dkms inside the rpm to follow recommended way in documentation. * Switch spec to using macro for buildroot instead of variable for consistency. * Change hardcoded paths to rpm macros * List out files instead of using a wild card. This Will help catch potential issues if files are missing or changed with new version releases. * Use systemd scriplets for handling systemd unit file * Use systemd preset file to enable displaylink.service by default * Remove call to enable dkms service since this is already enabled by policy on Fedora. * Add logrotate config file

The travis build flagged 'skip_cleanup' as being replaced with cleanup. The change is in a draft and I missed that. We need this back in for releases to work.

* Fixed typo in kernel package name

If 'make' is not installed on the system, then dkms is unable to build the evdi module. Resolves displaylink-rpm#139

Adding Fedora 33 as a target for Travis to build. Closes displaylink-rpm#150

There is a hard coded reference to libevdi.so.1.7.0. Since this version can change based on the currently released module, this patch changes this line to refer to the current version that is being built using the 'version' variable.

There was a typo in the Makefile. Change 'EDVI' reference to 'EVDI'.

It would appear that while in Fedora 30+ the rpmbuild has been able to handle the mangling of the shebang from '#!/bin/bash' to '#!/usr/bin/bash', the build for Fedora 33 is err'ing out. This is an attempt to correct this.

the evdi repository to support it.

Removed the requirement for 'ml' kernel when on RHEL / CentOS 8+ Added a requirement to have epel repository

- Update the module version in the necessary places. - Switch 'make devel' to point to the evdi/v1.7.x branch. This is the only branch that works with the current DisplayLink driver release (5.3.1). - Fix logic in spec for handling RHEL/CentOS 7 and below

For details, see dkms documentation.

Co-authored-by: user <user@x1>

This change brings in support for DisplayLink 5.4 and moves the devel builds to point to the current devel branch for evdi.

This will get around the rate limiting that docker hub is now doing on anonymous pulls.

* Add a 'Provides' to indicate the bundled library in this package. * Add a 'Requires' for libusbx * Use Fedora short names for license field

* Fixup for Fedora. * Add sources file. * Cleanup spec file. * README entry. * Update to the newer DisplayLink driver * Update sources with latest packages * Update spec to reflect Pavel's changes Co-authored-by: Michael L. Young <elgueromexicano@gmail.com>

DisplayLink manager 5.4. Since upstream evdi wasn't tagged properly we need to point to a commit id instead of the tag. Since upstream displaylink-rpm hasn't updated its tag, we still point to the old one.

ffgiff · 2021-04-21T21:45:52Z

Reopening this. I've been happily running my own akmods for well over a year now. Notwithstanding the current issues around the evdi 1.9.1 tagging, I'd like to get the ball rolling on this again and ultimately get it into rpmfusion.

ffgiff · 2021-04-26T21:21:45Z

Replaced by #166 as the rework has confused GitHub which produces diffs to the wrong commit on the master branch.

ffgiff mentioned this pull request Jun 25, 2019

Please join RPM Fusion project #76

Open

kwizart reviewed Jun 27, 2019

View reviewed changes

ffgiff closed this Feb 4, 2020

ffgiff force-pushed the master branch from ec6b372 to 4352525 Compare February 4, 2020 19:55

alhalama and others added 22 commits February 5, 2020 16:17

Updating evdi version to 1.6.4 from 1.6.3

316206f

Apply patches to get evdi to compile and install on kernel 5.4.

2702853

The patches were provided by abucodonosor and severach. See DisplayLink/evdi#172 (comment) See DisplayLink/evdi#172 (comment)

Merge pull request displaylink-rpm#118 from elguero/evdi1.6.4-kernel5…

6c010ca

….4-patch Apply patches to get evdi to compile and install on kernel 5.4.

Remove patches that are no longer needed now that evdi has merged in …

bf7ddd4

…support for kernels 5.4 and 5.5.

Fix uninstall of evdi driver

540dc70

When uninstalling, we need to remove the symbolic link first.

Fix wrong date in .spec

c3e711f

Merge pull request displaylink-rpm#131 from elguero/master

4210c5a

Fixes related to evdi devel branch changes

Update CI config settings (displaylink-rpm#134)

aef57ac

Fix CI change

d4a59d7

The travis build flagged 'skip_cleanup' as being replaced with cleanup. The change is in a draft and I missed that. We need this back in for releases to work.

Fixed typo in kernel package name. (displaylink-rpm#137)

fab90c4

* Fixed typo in kernel package name

Add the requirement to have 'make' installed

adf996b

If 'make' is not installed on the system, then dkms is unable to build the evdi module. Resolves displaylink-rpm#139

Adding Fedora 33 Target (displaylink-rpm#151)

2ac08f5

Adding Fedora 33 as a target for Travis to build. Closes displaylink-rpm#150

Fix typo in Makefile

3fcdadc

There was a typo in the Makefile. Change 'EDVI' reference to 'EVDI'.

Remove need to mangle shebang in displaylink.sh sleep script

6c572d0

It would appear that while in Fedora 30+ the rpmbuild has been able to handle the mangling of the shebang from '#!/bin/bash' to '#!/usr/bin/bash', the build for Fedora 33 is err'ing out. This is an attempt to correct this.

Add CentOS 8 to the Travis build. A recent commit was merged into

db1ff8b

the evdi repository to support it.

Update the spec file to improve experience on RHEL / CentOS

771ecb8

Removed the requirement for 'ml' kernel when on RHEL / CentOS 8+ Added a requirement to have epel repository

Adding an issue template to the repository

59ad114

Prevent adding symlink for weak-updates directory

5641980

For details, see dkms documentation.

sourcecodere and others added 7 commits March 9, 2021 16:54

Added secure boot instructions to README.md (displaylink-rpm#159)

268a60c

Co-authored-by: user <user@x1>

Add support for new DisplayLink driver and evdi

20b5aa1

This change brings in support for DisplayLink 5.4 and moves the devel builds to point to the current devel branch for evdi.

Add docker authentication to travis builds

7ecc58f

This will get around the rate limiting that docker hub is now doing on anonymous pulls.

Clean up references to old DisplayLink driver site

a60cf94

Adjustments to spec file

cfca076

* Add a 'Provides' to indicate the bundled library in this package. * Add a 'Requires' for libusbx * Use Fedora short names for license field

5.4 first look

2184c70

DisplayLink manager 5.4. Since upstream evdi wasn't tagged properly we need to point to a commit id instead of the tag. Since upstream displaylink-rpm hasn't updated its tag, we still point to the old one.

ffgiff reopened this Apr 21, 2021

ffgiff mentioned this pull request Apr 21, 2021

Travis builds fail for PRs #164

Closed

This was referenced Apr 26, 2021

EVDI kernel 5.11/5.12 support DisplayLink/evdi#275

Closed

Rebuilds improvement #125

Open

ffgiff mentioned this pull request Apr 26, 2021

Package 5.4.0 as akmod #166

Open

ffgiff closed this Apr 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Package evdi as akmod #97

Package evdi as akmod #97

ffgiff commented Jun 25, 2019

ssaavedra commented Jun 27, 2019

kwizart commented Jun 27, 2019

kwizart left a comment

kwizart Jun 27, 2019

kwizart Jun 27, 2019

kwizart Jun 27, 2019

ffgiff Jun 30, 2019

kwizart Jul 1, 2019

kwizart Jun 27, 2019

ffgiff Jun 30, 2019

kwizart Jun 27, 2019

kwizart Jun 27, 2019

ssaavedra Jun 28, 2019

kwizart Jun 28, 2019

ssaavedra Jun 28, 2019

kwizart Jul 1, 2019

kwizart Jun 27, 2019

ffgiff Jul 2, 2019

kwizart Jun 27, 2019

kwizart Jun 27, 2019

kwizart Jun 27, 2019

ssaavedra commented Jun 28, 2019

ffgiff commented Jun 30, 2019

ffgiff commented Apr 21, 2021

ffgiff commented Apr 26, 2021

Package evdi as akmod #97

Package evdi as akmod #97

Conversation

ffgiff commented Jun 25, 2019

ssaavedra commented Jun 27, 2019

kwizart commented Jun 27, 2019

kwizart left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ssaavedra commented Jun 28, 2019

ffgiff commented Jun 30, 2019

ffgiff commented Apr 21, 2021

ffgiff commented Apr 26, 2021