Page MenuHomeFreeBSD

Update kde4-workspace & co to 4.11.22 (last KDE4 releaese)
ClosedPublic

Authored by rezny on Mar 27 2017, 7:27 AM.

Details

Summary

This update resolves a bulid failure after the update of kdelibs4 to 4.14.30, which caused the configure step of kde4-workspace to fail. One new patch is needed for kde4-workspace itself to build, and there is one patch for ksysguardd that is not neccessary but nice to eliminate warnings. Since this is probablty the last update, take the opportunity to clean up the patches by renaming a couple to conform to current standards.

Test Plan

Builds for me on 11amd64

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

rezny created this revision.Mar 27 2017, 7:27 AM
tcberner added inline comments.
x11/plasma-scriptengine-python/pkg-plist
2 ↗(On Diff #26681)

This looks suspicious. Why are the PYO files no longer generated?

Could you revert the regeneration of the patches that do nothing but change the dates/names? It makes it kind of more bloated than needed.

tcberner added inline comments.Mar 27 2017, 7:45 AM
x11/plasma-scriptengine-ruby/Makefile
6 ↗(On Diff #26681)

If you bump the the version, could you please add kde-applications to the CATEGORIES and get rid of MASTER_SITES and DIST_SUBDIR and instead set KDE_APPLICATIONS_VERSION=15.08.0.

Same for the other places this occurs.

tcberner requested changes to this revision.Mar 27 2017, 7:55 AM
This revision now requires changes to proceed.Mar 27 2017, 7:55 AM
rezny updated this revision to Diff 26682.Mar 27 2017, 8:55 AM
rezny edited edge metadata.
rezny marked an inline comment as done.

Add kde-applications and override KDE_APPLICATIONS_VERSION as requested

rezny updated this revision to Diff 26683.Mar 27 2017, 8:58 AM

Update again with hopefully just the right files this time. Arcanist's failure to show the files it will act on until after it's done is VERY annoying...

mat added a comment.Mar 27 2017, 9:11 AM

Update again with hopefully just the right files this time. Arcanist's failure to show the files it will act on until after it's done is VERY annoying...

Well, you can use svn status it will tell you what files will arcanist run svn diff on.

Also, please, while regerating the patches to see if changes is needed is good, never commit changes that only affect the first two lines of a patch (the --- and +++ lines)

sysutils/ksysguardd/Makefile
22 ↗(On Diff #26683)

This should probably be WRKSRC_SUBDIR=ksysguard/${PORTNAME}

sysutils/ksysguardd/files/patch-CMakeLists.txt
1 ↗(On Diff #26683)

This is only header change, revert it.

sysutils/ksysguardd/files/patch-config-ksysguardd.h.cmake
1–2 ↗(On Diff #26683)

This is only header change, revert it.

x11/kde4-workspace/files/patch-CMakeLists.txt
1–2 ↗(On Diff #26683)

This is only a header change, revert.

x11/kde4-workspace/files/patch-ConfigureChecks.cmake
1 ↗(On Diff #26683)

This is only a header change, revert.

x11/kde4-workspace/files/patch-kdm-kfrontend-CMakeLists.txt
1 ↗(On Diff #26683)

This is only a header change, revert.

x11/kde4-workspace/files/patch-kdm_backend_client.c
1 ↗(On Diff #26683)

This is only a header change, revert.

x11/kde4-workspace/files/patch-kinfocenter
8 ↗(On Diff #26683)

This is only a header change, revert.

x11/kde4-workspace/files/patch-kinfocenter_Modules_pci_kpci.cpp
4 ↗(On Diff #26683)

This is only a header change, revert.

x11/kde4-workspace/files/patch-ksysguard_gui_SensorDisplayLib_FancyPlotter.cpp
9 ↗(On Diff #26683)

This is only a header change, revert.

x11/kde4-workspace/files/patch-ksysguard_gui_SensorDisplayLib_FancyPlotter.h
3 ↗(On Diff #26683)

This is only a header change, revert.

x11/kde4-workspace/files/patch-ksysguard_gui_SystemLoad2.sgrd
4 ↗(On Diff #26683)

This is only a header change, revert.

x11/kde4-workspace/files/patch-ksysguard_gui_ksysguard.cpp
5 ↗(On Diff #26683)

This is only a header change, revert.

x11/kde4-workspace/files/patch-ksysguard_gui_ksysguard.h
3 ↗(On Diff #26683)

This is only a header change, revert.

x11/kde4-workspace/files/patch-kwin_opengltest_CMakeLists.txt
1 ↗(On Diff #26683)

This is only a header change, revert.

x11/kde4-workspace/files/patch-plasma_desktop_toolboxes_desktoptoolbox.cpp
3 ↗(On Diff #26683)

This is only a header change, revert.

x11/kde4-workspace/files/patch-plasma_desktop_toolboxes_desktoptoolbox.h
3 ↗(On Diff #26683)

This is only a header change, revert.

x11/kde4-workspace/files/patch-powerdevil_daemon_backends_upower_xrandrbrightness.cpp
1 ↗(On Diff #26683)

This is only a header change, revert.

x11/kde4-workspace/files/patch-startkde.cmake
1 ↗(On Diff #26683)

This is only a header change, revert.

x11/kde4-workspace/files/patch-usbview
7 ↗(On Diff #26683)

This is only a header change, revert.

rezny added a comment.Mar 27 2017, 9:13 AM

Could you revert the regeneration of the patches that do nothing but change the dates/names? It makes it kind of more bloated than needed.

I'd appreciate firm guidelines on when is and isn't the right time to use makepatch. I had been told many times to just use makepatch, so I did anytime patches were changed/added, but now a few people say not to not touch the existing patches. So are patches that don't conform to be left until they must be touched, perhaps when they no longer apply? Should the patches with non-preferred names be left as-is entirely, renamed but content left as-is, or should those also remain untouched? I assume the reason to run makepatch is to keep things like the line numbers up to date so that it is easier to fix a patch when it does fail to apply. Perhaps makepatch should get a little smarter to not change the patch if it's only the dates that change? Or, even better idea, how about not putting any dates in the patch file? What purpose does the date in the patch file serve other than to bloat both the patch and all diffs thereof?

x11/plasma-scriptengine-python/pkg-plist
2 ↗(On Diff #26681)

I assume it is caused my the myriad of changes to cmake files in the last update of kdelibs4. It wasn't even possible to get through configure after that update.

Skimming commits, this one looks possibly related https://cgit.kde.org/kdelibs.git/commit/?h=KDE/4.14&id=016841aeb0b180981122085e9b1d49ae66951670

mat added a comment.Mar 27 2017, 9:30 AM

Could you revert the regeneration of the patches that do nothing but change the dates/names? It makes it kind of more bloated than needed.

I'd appreciate firm guidelines on when is and isn't the right time to use makepatch. I had been told many times to just use makepatch, so I did anytime patches were changed/added, but now a few people say not to not touch the existing patches. So are patches that don't conform to be left until they must be touched, perhaps when they no longer apply? Should the patches with non-preferred names be left as-is entirely, renamed but content left as-is, or should those also remain untouched? I assume the reason to run makepatch is to keep things like the line numbers up to date so that it is easier to fix a patch when it does fail to apply. Perhaps makepatch should get a little smarter to not change the patch if it's only the dates that change? Or, even better idea, how about not putting any dates in the patch file? What purpose does the date in the patch file serve other than to bloat both the patch and all diffs thereof?

A patch is composed of a header, and of hunks, each hunk has a header, and data. In this:

--- kinfocenter/Modules/base/info_fbsd.cpp.orig        2015-08-12 07:03:15 UTC
+++ kinfocenter/Modules/base/info_fbsd.cpp
@@ -15,8 +15,6 @@
  */
 
 #include "config-infocenter.h" // HAVE_DEVINFO_H
-#include <sys/types.h>
-#include <sys/sysctl.h>
 
 #ifdef HAVE_DEVINFO_H
 extern "C" {
@@ -60,9 +60,13 @@ bool GetInfo_IRQ(QTreeWidget* tree) {
 
 bool GetInfo_DMA(QTreeWidget* tree) {
 #ifdef HAVE_DEVINFO_H
-        /* Oh neat, current now has a neat little utility called devinfo */
         if (devinfo_init())
-        return false;
+                return false;
+
+        QStringList headers;
+        headers << i18n("DMA-Channel") << i18n("Used By");
+        tree->setHeaderLabels(headers);
+
         devinfo_foreach_rman(print_dma, tree);
         return true;

The header is:

--- kinfocenter/Modules/base/info_fbsd.cpp.orig        2015-08-12 07:03:15 UTC
+++ kinfocenter/Modules/base/info_fbsd.cpp

It is metadata, if it is the only thing that changes, it is unimportant, so, never include it.

This is a hunk:

@@ -60,9 +60,13 @@ bool GetInfo_IRQ(QTreeWidget* tree) {
 
 bool GetInfo_DMA(QTreeWidget* tree) {
 #ifdef HAVE_DEVINFO_H
-        /* Oh neat, current now has a neat little utility called devinfo */
         if (devinfo_init())
-        return false;
+                return false;
+
+        QStringList headers;
+        headers << i18n("DMA-Channel") << i18n("Used By");
+        tree->setHeaderLabels(headers);
+
         devinfo_foreach_rman(print_dma, tree);
         return true;

It starts with a @@ -xx1,yy1 +xx2,yy2 @@ <function name> line, which is its header, it says where in the file it is supposed to be applied (patch will, of course, look around in case things move).

If you want some details about what the numbers are:

  • xx1 : the line number the hunk starts to apply to
  • yy1 : the number of lines the hunk applies to
  • xx2 : the line number the hunk starts after it is applied
  • yy2 : the number of lines the hunk generates

If the numbers in the hunk header change, they are important, because it tells patch where to look for the lines to patch in the file, so even if they are the only thing that change, the patch must be committed. The <function name> is generated by diff using the C function name the patch is in, it is only helpful for us humans, when reading the patch, to figure out in an easier way where the patch is being applied. It is metadata, and if it is the only thing that changes, it should be discarded.

The you have the actual content of the hunk, every change in it is important.

  • lines starting with a single space are context, and they help patch figure out where to patch when the file is updated and things have moved around.
  • lines starting with a minus sign are lines that are supposed to go away.
  • lines starting with a plus sign are lines that are going in.
rezny added a comment.Mar 27 2017, 9:33 AM
In D10148#209718, @mat wrote:

Update again with hopefully just the right files this time. Arcanist's failure to show the files it will act on until after it's done is VERY annoying...

Well, you can use svn status it will tell you what files will arcanist run svn diff on.

Actually, that is not true, unless there has been a change of behavior from the first time I used (and was thoroughly surprised by) arcanist. When not in the root of the repo, svn status shows what has changed in the current directory and sub-directories while ignoring parent and sibling directories, whereas arc diff grabs any change in the repo regardless of current directory. So, I must always give some list of paths/files to arcanist, different from svn. That is only a slight annoyance, though I'd be very pleased to hear there is some way to change that behavior (I admit to not having looked closely at the available options).

The real problem is that it does not show the list of files below the message template as svn does when composing a commit message, so if I have made a mistake and over-selected (say, a stray space between a path and an asterisk), I do not know until it is too late. That's just poor UI, and while running svn status with the same list is a work-around, it is just silly to have to rely on that extra step. Speaking of poor UI, I really need to figure out how to make the diff view and input area text on phabricator pages lager than 6pt. The summary, comments, and file names between diff sections are a comfortable size, but the most important part, the actual diff content and anything I type, is half the size and weight. At least I see some icons instead of the invalid unicode characters today. Maybe I'll try to improve arcanist when I get through the rest of the to-dos, but no promises (it's PHP).

mat added a comment.Mar 27 2017, 9:38 AM
In D10148#209718, @mat wrote:

Update again with hopefully just the right files this time. Arcanist's failure to show the files it will act on until after it's done is VERY annoying...

Well, you can use svn status it will tell you what files will arcanist run svn diff on.

Actually, that is not true, unless there has been a change of behavior from the first time I used (and was thoroughly surprised by) arcanist. When not in the root of the repo, svn status shows what has changed in the current directory and sub-directories while ignoring parent and sibling directories, whereas arc diff grabs any change in the repo regardless of current directory. So, I must always give some list of paths/files to arcanist, different from svn. That is only a slight annoyance, though I'd be very pleased to hear there is some way to change that behavior (I admit to not having looked closely at the available options).

Let me rephrase what I said, you can see what arcanist will use by doing cd <rootdir of checkout>; svn status

The real problem is that it does not show the list of files below the message template as svn does when composing a commit message, so if I have made a mistake and over-selected (say, a stray space between a path and an asterisk), I do not know until it is too late. That's just poor UI, and while running svn status with the same list is a work-around, it is just silly to have to rely on that extra step. Speaking of poor UI, I really need to figure out how to make the diff view and input area text on phabricator pages lager than 6pt. The summary, comments, and file names between diff sections are a comfortable size, but the most important part, the actual diff content and anything I type, is half the size and weight. At least I see some icons instead of the invalid unicode characters today. Maybe I'll try to improve arcanist when I get through the rest of the to-dos, but no promises (it's PHP).

I'm going to say that I don't use svn at all unless I am committing a patch that moves files, and then, only to do the actual commit to the ports tree.
I exclusively use git for all my ports work, and it does not have that problem wrt arcanist, because you put each thing you work on a separate branch, and you actually commit the files before running arc diff.

rezny added a comment.Mar 27 2017, 9:39 AM

Mat, thanks for the thorough explanation! The details of the line numbers is a known, I regularly adjust those by hand. What I needed is basically this: commit data changes (hunk contents and offsets), discard metadata changes (dates, offset trailers, etc).

I'm going to say that I don't use svn at all unless I am committing a patch that moves files, and then, only to do the actual commit to the ports tree.
I exclusively use git for all my ports work, and it does not have that problem wrt arcanist, because you put each thing you work on a separate branch, and you actually commit the files before running arc diff.

Sparse svn clones with only the affected ports works also just great :) -- and it's also extremely fast, as a bonus.

rezny marked 19 inline comments as done.Mar 27 2017, 9:58 AM
rezny updated this revision to Diff 26684.Mar 27 2017, 9:59 AM

Remove patch files with metadata-only changes

Thanks for the patch-reverts, now this looks much nicer.

rezny marked an inline comment as done.Mar 27 2017, 10:10 AM
rezny updated this revision to Diff 26685.Mar 27 2017, 10:11 AM

Switch to WRKSRC_SUBDIR as suggested

tcberner added inline comments.Mar 27 2017, 10:23 AM
x11/kde4-workspace/distinfo
1 ↗(On Diff #26685)

This needs to be regenerated after the switch to CATEGORIES=kde-applications.

SHA256 (KDE/applications/15.08.0/kde-workspace-4.11.22.tar.xz) = f035334e843d67ee88551ae9e6c5f64bf7b1edfe311b12501575fe74be0b03b7
SIZE (KDE/applications/15.08.0/kde-workspace-4.11.22.tar.xz) = 13553668
rakuco added a subscriber: rakuco.Mar 27 2017, 10:36 AM

Apologies for the breakage in the first place, I should have asked for an exp-run before landing the kdelibs4 update. The .pyo changes in the pkg-plists should be reverted: I'm testing a new build here reintroducing patch to kdelibs4 that makes sure those files are built and installed.

I'm testing a new build here reintroducing patch to kdelibs4 that makes sure those files are built and installed.

Landed in rP437020.

rezny updated this revision to Diff 26691.Mar 27 2017, 2:15 PM

Refresh distinfo and drop the pkg-plist changes after confirming the .pyo files are back with the last kdelibs4 update.

rezny marked 3 inline comments as done.Mar 27 2017, 2:15 PM
rakuco accepted this revision.Mar 27 2017, 2:21 PM

lgtm, thank you.

tcberner accepted this revision.Mar 27 2017, 2:35 PM

same here.

rakuco requested changes to this revision.Mar 27 2017, 3:47 PM

Unbreaking the port was quite urgent, so I landed some backports as well as one of the CMake patches in rP437053 to get 4.11.21 to build with the new kdelibs. Please rebase your patch so that it works again.

This revision now requires changes to proceed.Mar 27 2017, 3:47 PM
rezny updated this revision to Diff 26696.Mar 27 2017, 4:14 PM
rezny edited edge metadata.

Refresh after rakuco's last commit: Drop the -lXss fix he already landed, delete the two new patches that are part of this release, and rename (but not change content of) the three files with legacy names.

swills accepted this revision.Mar 29 2017, 2:59 PM

Approved

This revision is now accepted and ready to land.Mar 29 2017, 2:59 PM
This revision was automatically updated to reflect the committed changes.