Page MenuHomeFreeBSD
Differential D8378

Get rid of the QT_FOODIR plist subs
ClosedPublic
Actions

Authored by tcberner on Oct 28 2016, 7:32 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Apr 15, 4:57 PM
Unknown Object (File)
Mon, Apr 14, 7:25 AM
Unknown Object (File)
Thu, Mar 20, 1:35 AM
Unknown Object (File)
Mar 12 2025, 12:23 AM
Unknown Object (File)
Mar 10 2025, 10:29 PM
Unknown Object (File)
Feb 21 2025, 6:29 PM
Unknown Object (File)
Jan 24 2025, 10:43 AM
Unknown Object (File)
Jan 20 2025, 12:43 AM
View All Files
Subscribers
adridg

Details

Reviewers
rakuco
mat
Group Reviewers
O5: Ports Framework
portmgr
kde
Commits
rP427356: Stop exporting QT_BINDIR and QT_LIBDIR to PLIST_SUB.
Summary

Very few non-Qt-dist ports actually needed to use the plist subs for
QT_BINDIR, QT_LIBDIR and so on. However, as they were exported by bsd.qt.mk,
make makeplist freely replaced every occurence of bin or lib with
said subs.

There is however no straight-forward way to decide whether a sub is sensible or
not without manually looking at the created plist and the projects buildscripts,
followed by thinking about it (does it actually install to `where the Qt
binaries go`, or just bin/).

Therefore simply get rid of all the QT_FOODIR subs.

Note1: Maybe this is too aggressive, and we could only rid ourselves of
QT_BINDIR and QT_LIBDIR as they are the most egregiously misused ones.
But, I think ripping them all out at once is the cleanest way to handle this.

Note2: As there are no changes apart from plist rewrites, I don't think any version
needs to be bumped.

Note3: It could be, that some ports (or slaveports) that have QT4&QT5 options
and used %%QT_FOO%% are broken by this -- they need to define the PLIST_SUB
now manually, like multimedia/gstreamer1-qt[45] and math/vtk6.

[diff created using 'svn diff --diff-cmd diff -x -U99999', as arc choked on it]

Test Plan
  • This of course will require an exp-run :)
  • I test built all affected ports -- I did however not chase options.

Diff Detail

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

Event Timeline

tcberner updated this revision to Diff 21781.Oct 28 2016, 7:32 PM
tcberner retitled this revision from to Get rid of the QT_FOODIR plist subs.
tcberner updated this object.
tcberner edited the test plan for this revision. (Show Details)
tcberner added reviewers: kde, mat, rakuco.
tcberner set the repository for this revision to rP FreeBSD ports repository.
tcberner added a subscriber: adridg.
Herald added a reviewer: portmgr. · View Herald TranscriptOct 28 2016, 7:32 PM
tcberner added a comment.Oct 28 2016, 7:38 PM

As a "lighter" version of this diff, we could keep exporting the QT_FOODIR subs whenever QT_DIST is defined. That way the qt[45]-ports would not be affected by this, and the diff would be more managable.

mat edited edge metadata.Oct 28 2016, 11:05 PM

Mmmm, I was only talking about removing QT_BINDIR and QT_LIBDIR, because both are way too short. The other are fine.

tcberner added a comment.Oct 29 2016, 6:58 AM

Misunderstandings are fun, aren't they :D

In that case, the whole thing is moot.

With qt-5.7 I plan to move the qt5 libraries to lib/qt5. QT_LIBDIR therefore becomes lib/qt5 and QT_BINDIR for qt4 and qt5 will become ${QT_LIBDIR}/bin [or maybe bin/${_QT_RELMAME}].

So they won't get plist-subbed in anymore then if not actually needed anyway.

mat added a comment.Oct 29 2016, 9:12 AM

I don't quite like having QT_BINDIR not being in PREFIX/bin, it means its content will not be in PATH, and not readily accessible. (unless you add symlinks to PREFIX/bin, in which case I don't really see the point of them being some place else anyway.)

tcberner added a comment.Oct 29 2016, 9:17 AM

misc/qtchooser will be used which installs to ${LOCALBASE}/bin and selects the proper qmake /uic/moc/foo depending on the environment.

mat added a comment.Oct 30 2016, 9:54 AM

So, this means people working on QT apps, when they want to build with qt4 or qt5, they'll have to sudo and run some software ?

tcberner added a comment.Oct 30 2016, 11:18 AM

misc/qtchooser installs wrapper binaries that select the correct non-wrapped binary depending on the value of QT_SELECT (which has to be set to an installed qt-profile [1]).

% env QT_SELECT=qt4 qmake -v
QMake version 2.01a
Using Qt version 4.8.7 in /usr/local/lib/qt4

% env QT_SELECT=qt5 qmake -v
QMake version 3.0
Using Qt version 5.7.1 in /usr/local/lib/qt5

This means with that change, users could set their default QT_SELECT in their shell-profile, and qmake would simply give them the correct one. Unlike now, where you have to decide whether you want /usr/local/bin/qmake-qt4 or /usr/local/lib/qt5/bin/qmake.

For the ports, bsd.qt.mk/qt.mk will just choose the correct profile.

This also allows people that work on Qt itself to have multiple version of Qt installed simultaneously and build against the proper one by providing a new qt-foo profile [1].

[1] The profiles currently installed are

% cat /usr/local/etc/xdg/qtchooser/qt4.conf
/usr/local/lib/qt4/bin
/usr/local/lib/qt4
% cat /usr/local/etc/xdg/qtchooser/qt5.conf
/usr/local/lib/qt5/bin
/usr/local/lib/qt5

A person working on Qt could simply add a profile ala

% cat qt58.conf 
/home/Tobias/qt-devel/qt-5.8/bin
/home/Tobias/qt-devel/qt-5.8/lib

and use it via env QT_SELECT=qt58 qmake.

tcberner updated this revision to Diff 22068.Nov 8 2016, 1:13 AM
tcberner edited the test plan for this revision. (Show Details)
tcberner edited edge metadata.

Let's do this completely minimal...

Only export QT_BINDIR and QT_LIBDIR if
QT_DIST is set.

Thereby all the Qt-ports themselves are unaffected by
this change, and non Qt-ports will no longer pick them
up by mistake.

Owners added a reviewer: O5: Ports Framework.Nov 8 2016, 1:13 AM
Herald edited edge metadata. · View Herald TranscriptNov 8 2016, 1:13 AM
tcberner added a comment.Nov 26 2016, 1:22 PM

ping @mat pong @rakuco

rakuco accepted this revision.Nov 26 2016, 1:54 PM
rakuco edited edge metadata.

Looks fine to me, but I'd drop the whitespace changes in bsd.qt.mk. They make the patch bigger, the history more difficult to navigate through and moving to Uses/qt.mk will make these changes short-lived.

mat added a comment.Nov 28 2016, 3:36 PM

Yes, drop the whitespace change, you will be able to use Tools/scripts/indent_make_if.pl when moving to USES=qt.

tcberner updated this revision to Diff 22579.Nov 28 2016, 5:08 PM
tcberner edited edge metadata.

Get rid of the changes made by Tools/scripts/indent_make_if.pl .

Owners edited edge metadata.Nov 28 2016, 5:08 PM
Herald edited edge metadata. · View Herald TranscriptNov 28 2016, 5:08 PM
rakuco accepted this revision.Nov 28 2016, 8:32 PM
rakuco edited edge metadata.

lgtm!

tcberner updated this revision to Diff 22589.Nov 28 2016, 9:26 PM
tcberner edited edge metadata.

An other grep for QT_BINDIR and QT_LIBDIR brought some new wrong uses.
They are fixed now too.

Owners edited edge metadata.Nov 28 2016, 9:26 PM
Herald edited edge metadata. · View Herald TranscriptNov 28 2016, 9:26 PM
mat accepted this revision.Nov 28 2016, 9:56 PM
mat edited edge metadata.

Macro shipit:

This revision is now accepted and ready to land.Nov 28 2016, 9:56 PM
Closed by commit rP427356: Stop exporting QT_BINDIR and QT_LIBDIR to PLIST_SUB. (authored by tcberner). · Explain WhyNov 29 2016, 6:20 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSizePackages
head/
Mk/
9 linesO5: Ports Framework
cad/
qelectrotech/
20 lines
devel/
liblxqt/
6 lines
libqtxdg/
12 lines
qscintilla2-qt5/
8 lines
qscintilla2/
8 lines
qt4-libqtassistantclient/
10 lines
graphics/
gle-graphics/
8 lines
math/
vtk6/
2 lines
multimedia/
dvbcut/
1 line
gstreamer-qt4/
4 lines
phonon/
8 lines
qt4-mobility/
116 lines
net-p2p/
bitcoin/
2 lines
litecoin/
2 lines
sysutils/
bareos-traymonitor/
2 lines
qzeitgeist/
4 lines
x11-themes/
qt5-style-plugins/
2 lines

Diff 22593

View Options

head/Mk/bsd.qt.mk

Context not available.
#include <linux/dcache.h> #include <linux/dcache.h>
#include <linux/capability.h> #include <linux/capability.h>
#include <linux/wait_bit.h> #include <linux/wait_bit.h>
#include <linux/kernel.h>
#include <linux/mutex.h>
struct module; struct module;
struct kiocb; struct kiocb;
Context not available.
static inline int static inline int
simple_open(struct inode *inode, struct file *filp) simple_open(struct inode *inode, struct file *filp)
{ {
filp->private_data = inode->i_private;
return 0; return 0;
} }
Context not available.
return (-ESPIPE); return (-ESPIPE);
} }
#define default_llseek no_llseek
hselaskyUnsubmitted
Done
Inline Actions

Can you make this a static inline function, instead of using a macro?

static inline loff_t
default_llseek(....)

This avoids the macro, which may match more than it should.

hselasky: Can you make this a static inline function, instead of using a macro? static inline loff_t…
static inline loff_t static inline loff_t
noop_llseek(struct linux_file *file, loff_t offset, int whence) noop_llseek(struct linux_file *file, loff_t offset, int whence)
Context not available.
return (file->f_op->mmap(file, vma)); return (file->f_op->mmap(file, vma));
} }
static inline void
i_size_write(struct inode *inode, loff_t i_size)
{
/* inode->i_size = i_size; */
markjUnsubmitted
Done
Inline Actions

Why is this commented out?

markj: Why is this commented out?
hselaskyUnsubmitted
Done
Inline Actions

Please remove this comment.

hselasky: Please remove this comment.
}
/*
* simple_read_from_buffer: copy data from kernel-space origin
* buffer into user-space destination buffer
*
* @dest: destination buffer
* @read_size: number of bytes to be transferred
* @ppos: starting transfer position pointer
* @orig: origin buffer
* @buf_size: size of destination and origin buffers
*
* Return value:
* On success, total bytes copied with *ppos incremented accordingly.
* On failure, negative value.
*/
static inline ssize_t
simple_read_from_buffer(void __user *dest, size_t read_size, loff_t *ppos,
void *orig, size_t buf_size)
{
void *read_pos = ((char *) orig) + *ppos;
size_t buf_remain = buf_size - *ppos;
ssize_t num_read;
if (buf_remain < 0 || buf_remain > buf_size)
return -EINVAL;
if (read_size > buf_remain)
read_size = buf_remain;
/* copy_to_user returns number of bytes NOT read */
num_read = read_size - copy_to_user(dest, read_pos, read_size);
if (num_read == 0)
return -EFAULT;
*ppos += num_read;
return (num_read);
}
MALLOC_DECLARE(M_LSATTR);
#define DEFINE_SIMPLE_ATTRIBUTE(__fops, __get, __set, __fmt) \
static inline int \
__fops ## _open(struct inode *inode, struct file *filp) \
{ \
return (simple_attr_open(inode, filp, __get, __set, __fmt)); \
} \
static const struct file_operations __fops = { \
.owner = THIS_MODULE, \
.open = __fops ## _open, \
.release = simple_attr_release, \
.read = simple_attr_read, \
.write = simple_attr_write, \
.llseek = no_llseek \
}
int simple_attr_open(struct inode *inode, struct file *filp,
int (*get)(void *, uint64_t *), int (*set)(void *, uint64_t),
const char *fmt);
int simple_attr_release(struct inode *inode, struct file *filp);
ssize_t simple_attr_read(struct file *filp, const char *buf, size_t read_size, loff_t *ppos);
ssize_t simple_attr_write(struct file *filp, const char *buf, size_t write_size, loff_t *ppos);
#endif /* _LINUXKPI_LINUX_FS_H_ */ #endif /* _LINUXKPI_LINUX_FS_H_ */
Context not available.
View Options

head/cad/qelectrotech/pkg-plist

Context not available.
#include <linux/dcache.h> #include <linux/dcache.h>
#include <linux/capability.h> #include <linux/capability.h>
#include <linux/wait_bit.h> #include <linux/wait_bit.h>
#include <linux/kernel.h>
#include <linux/mutex.h>
struct module; struct module;
struct kiocb; struct kiocb;
Context not available.
static inline int static inline int
simple_open(struct inode *inode, struct file *filp) simple_open(struct inode *inode, struct file *filp)
{ {
filp->private_data = inode->i_private;
return 0; return 0;
} }
Context not available.
return (-ESPIPE); return (-ESPIPE);
} }
#define default_llseek no_llseek
hselaskyUnsubmitted
Done
Inline Actions

Can you make this a static inline function, instead of using a macro?

static inline loff_t
default_llseek(....)

This avoids the macro, which may match more than it should.

hselasky: Can you make this a static inline function, instead of using a macro? static inline loff_t…
static inline loff_t static inline loff_t
noop_llseek(struct linux_file *file, loff_t offset, int whence) noop_llseek(struct linux_file *file, loff_t offset, int whence)
Context not available.
return (file->f_op->mmap(file, vma)); return (file->f_op->mmap(file, vma));
} }
static inline void
i_size_write(struct inode *inode, loff_t i_size)
{
/* inode->i_size = i_size; */
markjUnsubmitted
Done
Inline Actions

Why is this commented out?

markj: Why is this commented out?
hselaskyUnsubmitted
Done
Inline Actions

Please remove this comment.

hselasky: Please remove this comment.
}
/*
* simple_read_from_buffer: copy data from kernel-space origin
* buffer into user-space destination buffer
*
* @dest: destination buffer
* @read_size: number of bytes to be transferred
* @ppos: starting transfer position pointer
* @orig: origin buffer
* @buf_size: size of destination and origin buffers
*
* Return value:
* On success, total bytes copied with *ppos incremented accordingly.
* On failure, negative value.
*/
static inline ssize_t
simple_read_from_buffer(void __user *dest, size_t read_size, loff_t *ppos,
void *orig, size_t buf_size)
{
void *read_pos = ((char *) orig) + *ppos;
size_t buf_remain = buf_size - *ppos;
ssize_t num_read;
if (buf_remain < 0 || buf_remain > buf_size)
return -EINVAL;
if (read_size > buf_remain)
read_size = buf_remain;
/* copy_to_user returns number of bytes NOT read */
num_read = read_size - copy_to_user(dest, read_pos, read_size);
if (num_read == 0)
return -EFAULT;
*ppos += num_read;
return (num_read);
}
MALLOC_DECLARE(M_LSATTR);
#define DEFINE_SIMPLE_ATTRIBUTE(__fops, __get, __set, __fmt) \
static inline int \
__fops ## _open(struct inode *inode, struct file *filp) \
{ \
return (simple_attr_open(inode, filp, __get, __set, __fmt)); \
} \
static const struct file_operations __fops = { \
.owner = THIS_MODULE, \
.open = __fops ## _open, \
.release = simple_attr_release, \
.read = simple_attr_read, \
.write = simple_attr_write, \
.llseek = no_llseek \
}
int simple_attr_open(struct inode *inode, struct file *filp,
int (*get)(void *, uint64_t *), int (*set)(void *, uint64_t),
const char *fmt);
int simple_attr_release(struct inode *inode, struct file *filp);
ssize_t simple_attr_read(struct file *filp, const char *buf, size_t read_size, loff_t *ppos);
ssize_t simple_attr_write(struct file *filp, const char *buf, size_t write_size, loff_t *ppos);
#endif /* _LINUXKPI_LINUX_FS_H_ */ #endif /* _LINUXKPI_LINUX_FS_H_ */
Context not available.
View Options

head/devel/liblxqt/pkg-plist

Context not available.
#include <linux/dcache.h> #include <linux/dcache.h>
#include <linux/capability.h> #include <linux/capability.h>
#include <linux/wait_bit.h> #include <linux/wait_bit.h>
#include <linux/kernel.h>
#include <linux/mutex.h>
struct module; struct module;
struct kiocb; struct kiocb;
Context not available.
static inline int static inline int
simple_open(struct inode *inode, struct file *filp) simple_open(struct inode *inode, struct file *filp)
{ {
filp->private_data = inode->i_private;
return 0; return 0;
} }
Context not available.
return (-ESPIPE); return (-ESPIPE);
} }
#define default_llseek no_llseek
hselaskyUnsubmitted
Done
Inline Actions

Can you make this a static inline function, instead of using a macro?

static inline loff_t
default_llseek(....)

This avoids the macro, which may match more than it should.

hselasky: Can you make this a static inline function, instead of using a macro? static inline loff_t…
static inline loff_t static inline loff_t
noop_llseek(struct linux_file *file, loff_t offset, int whence) noop_llseek(struct linux_file *file, loff_t offset, int whence)
Context not available.
return (file->f_op->mmap(file, vma)); return (file->f_op->mmap(file, vma));
} }
static inline void
i_size_write(struct inode *inode, loff_t i_size)
{
/* inode->i_size = i_size; */
markjUnsubmitted
Done
Inline Actions

Why is this commented out?

markj: Why is this commented out?
hselaskyUnsubmitted
Done
Inline Actions

Please remove this comment.

hselasky: Please remove this comment.
}
/*
* simple_read_from_buffer: copy data from kernel-space origin
* buffer into user-space destination buffer
*
* @dest: destination buffer
* @read_size: number of bytes to be transferred
* @ppos: starting transfer position pointer
* @orig: origin buffer
* @buf_size: size of destination and origin buffers
*
* Return value:
* On success, total bytes copied with *ppos incremented accordingly.
* On failure, negative value.
*/
static inline ssize_t
simple_read_from_buffer(void __user *dest, size_t read_size, loff_t *ppos,
void *orig, size_t buf_size)
{
void *read_pos = ((char *) orig) + *ppos;
size_t buf_remain = buf_size - *ppos;
ssize_t num_read;
if (buf_remain < 0 || buf_remain > buf_size)
return -EINVAL;
if (read_size > buf_remain)
read_size = buf_remain;
/* copy_to_user returns number of bytes NOT read */
num_read = read_size - copy_to_user(dest, read_pos, read_size);
if (num_read == 0)
return -EFAULT;
*ppos += num_read;
return (num_read);
}
MALLOC_DECLARE(M_LSATTR);
#define DEFINE_SIMPLE_ATTRIBUTE(__fops, __get, __set, __fmt) \
static inline int \
__fops ## _open(struct inode *inode, struct file *filp) \
{ \
return (simple_attr_open(inode, filp, __get, __set, __fmt)); \
} \
static const struct file_operations __fops = { \
.owner = THIS_MODULE, \
.open = __fops ## _open, \
.release = simple_attr_release, \
.read = simple_attr_read, \
.write = simple_attr_write, \
.llseek = no_llseek \
}
int simple_attr_open(struct inode *inode, struct file *filp,
int (*get)(void *, uint64_t *), int (*set)(void *, uint64_t),
const char *fmt);
int simple_attr_release(struct inode *inode, struct file *filp);
ssize_t simple_attr_read(struct file *filp, const char *buf, size_t read_size, loff_t *ppos);
ssize_t simple_attr_write(struct file *filp, const char *buf, size_t write_size, loff_t *ppos);
#endif /* _LINUXKPI_LINUX_FS_H_ */ #endif /* _LINUXKPI_LINUX_FS_H_ */
Context not available.
View Options

head/devel/libqtxdg/pkg-plist

Context not available.
#include <linux/dcache.h> #include <linux/dcache.h>
#include <linux/capability.h> #include <linux/capability.h>
#include <linux/wait_bit.h> #include <linux/wait_bit.h>
#include <linux/kernel.h>
#include <linux/mutex.h>
struct module; struct module;
struct kiocb; struct kiocb;
Context not available.
static inline int static inline int
simple_open(struct inode *inode, struct file *filp) simple_open(struct inode *inode, struct file *filp)
{ {
filp->private_data = inode->i_private;
return 0; return 0;
} }
Context not available.
return (-ESPIPE); return (-ESPIPE);
} }
#define default_llseek no_llseek
hselaskyUnsubmitted
Done
Inline Actions

Can you make this a static inline function, instead of using a macro?

static inline loff_t
default_llseek(....)

This avoids the macro, which may match more than it should.

hselasky: Can you make this a static inline function, instead of using a macro? static inline loff_t…
static inline loff_t static inline loff_t
noop_llseek(struct linux_file *file, loff_t offset, int whence) noop_llseek(struct linux_file *file, loff_t offset, int whence)
Context not available.
return (file->f_op->mmap(file, vma)); return (file->f_op->mmap(file, vma));
} }
static inline void
i_size_write(struct inode *inode, loff_t i_size)
{
/* inode->i_size = i_size; */
markjUnsubmitted
Done
Inline Actions

Why is this commented out?

markj: Why is this commented out?
hselaskyUnsubmitted
Done
Inline Actions

Please remove this comment.

hselasky: Please remove this comment.
}
/*
* simple_read_from_buffer: copy data from kernel-space origin
* buffer into user-space destination buffer
*
* @dest: destination buffer
* @read_size: number of bytes to be transferred
* @ppos: starting transfer position pointer
* @orig: origin buffer
* @buf_size: size of destination and origin buffers
*
* Return value:
* On success, total bytes copied with *ppos incremented accordingly.
* On failure, negative value.
*/
static inline ssize_t
simple_read_from_buffer(void __user *dest, size_t read_size, loff_t *ppos,
void *orig, size_t buf_size)
{
void *read_pos = ((char *) orig) + *ppos;
size_t buf_remain = buf_size - *ppos;
ssize_t num_read;
if (buf_remain < 0 || buf_remain > buf_size)
return -EINVAL;
if (read_size > buf_remain)
read_size = buf_remain;
/* copy_to_user returns number of bytes NOT read */
num_read = read_size - copy_to_user(dest, read_pos, read_size);
if (num_read == 0)
return -EFAULT;
*ppos += num_read;
return (num_read);
}
MALLOC_DECLARE(M_LSATTR);
#define DEFINE_SIMPLE_ATTRIBUTE(__fops, __get, __set, __fmt) \
static inline int \
__fops ## _open(struct inode *inode, struct file *filp) \
{ \
return (simple_attr_open(inode, filp, __get, __set, __fmt)); \
} \
static const struct file_operations __fops = { \
.owner = THIS_MODULE, \
.open = __fops ## _open, \
.release = simple_attr_release, \
.read = simple_attr_read, \
.write = simple_attr_write, \
.llseek = no_llseek \
}
int simple_attr_open(struct inode *inode, struct file *filp,
int (*get)(void *, uint64_t *), int (*set)(void *, uint64_t),
const char *fmt);
int simple_attr_release(struct inode *inode, struct file *filp);
ssize_t simple_attr_read(struct file *filp, const char *buf, size_t read_size, loff_t *ppos);
ssize_t simple_attr_write(struct file *filp, const char *buf, size_t write_size, loff_t *ppos);
#endif /* _LINUXKPI_LINUX_FS_H_ */ #endif /* _LINUXKPI_LINUX_FS_H_ */
Context not available.
View Options

head/devel/qscintilla2-qt5/pkg-plist

Loading...
View Options

head/devel/qscintilla2/pkg-plist

Loading...
View Options

head/devel/qt4-libqtassistantclient/pkg-plist

Loading...
View Options

head/graphics/gle-graphics/pkg-plist

Loading...
View Options

head/math/vtk6/pkg-plist

Loading...
View Options

head/multimedia/dvbcut/pkg-plist

Loading...
View Options

head/multimedia/gstreamer-qt4/pkg-plist

Loading...
View Options

head/multimedia/phonon/pkg-plist

Loading...
View Options

head/multimedia/qt4-mobility/pkg-plist

Loading...
View Options

head/net-p2p/bitcoin/Makefile

Loading...
View Options

head/net-p2p/litecoin/Makefile

Loading...
View Options

head/sysutils/bareos-traymonitor/pkg-plist

Loading...
View Options

head/sysutils/qzeitgeist/pkg-plist

Loading...
View Options

head/x11-themes/qt5-style-plugins/pkg-plist

Loading...