TEST_LOG semantics
This exists only so that individual ports may log. Tweaking it anywhere is awkward (and will be more so with SEPARATE_BUILD). I just checked that the current ports tree does not abuse this. Okay ? Index: bsd.port.mk.5 === RCS file: /cvs/src/share/man/man5/bsd.port.mk.5,v retrieving revision 1.474 diff -u -p -r1.474 bsd.port.mk.5 --- bsd.port.mk.5 31 Jan 2018 17:43:33 - 1.474 +++ bsd.port.mk.5 3 Apr 2018 21:09:43 - @@ -2921,6 +2921,7 @@ if port needs human interaction to run i if the tests need an active X11 display to work. .It Ev TEST_LOG Command used to log the results of regression tests to TEST_LOGFILE. +Read-only. .It Ev TEST_LOGFILE Log file containing the results of regression tests. .It Ev TEST_TARGET
Re: manpage text width
On Fri, Mar 30, 2018 at 01:57:43AM +0200, Ingo Schwarze wrote: > I do *NOT* want to add SIGWINCH signal handling to man(1) to abort > less(1), reformat, and respawn less(1) in that case. That kind of > magic would be over the top, and SIGWINCH is an abomination in the > first place. Why on earth would you think so ?... I've been dealing with SIGWINCH without real issues in a few programs and it works really nice.
Re: ld.lld/ld.bfd difference in -rpath management
On Sat, Oct 27, 2018 at 01:01:10PM +0200, Mark Kettenis wrote: > > Correct. More recent GNU linkers behave the same way as lld, which is > why I think the change in behaviour is a good thing. > > It is possible to make ld.lld emit DT_RPATH by passing > -Wl,-disable-new-dtags. And if you pass -Wl,-enable-new-dtags, our > ld.bfd will emit DT_RUNPATH (although it will also emit DT_RPATH in > that case). Maybe we should revisit the defaults for ld.bfd on this. As far as libtool is concerned, it won't care at all. Diagnosing the problem was the hard part.
Re: tcsh -- build without sbrk
On Mon, Nov 05, 2018 at 08:43:56AM -0500, Daniel Dickman wrote: > > > > On Nov 5, 2018, at 8:30 AM, Marc Espie wrote: > > > > Or we could just finally remove brk and sbrk from the libc ? > > > > > > you won’t get very far. they are still needed in base (gcc, clang, mkhybrid). The big question is, are they still *needed* or do those tools just *use them if they find them*.
Re: tcsh -- build without sbrk
On Mon, Nov 05, 2018 at 09:15:28AM -0500, Daniel Dickman wrote: >gcc uses them for precompiled headers (PCH) which is a local diff added >by kurt@ in 2009. its likely nothing in base uses PCH but i don't know >what in ports needs this: This has always been a mess. I suspect it's not really important these days because pch only make sense for large C++ codebases, which are definitely not going to be happy with the gcc from base anyway. There is also some snippet using sbrk to avoid malloc in gmon.c. That might be more of an issue... >[3]https://github.com/openbsd/src/commit/cfee5d1 > >choices there would be to disable PCH support or maybe there's a >different way to reimplement without brk/sbrk. >clang looks like they have a HAVE_SBRK ifdef or something like that. so >usage can likely be turned off but i don't know this codebase that well >so that's just an assumption. Yep, I'll have to look.
Re: tcsh -- build without sbrk
On Mon, Nov 05, 2018 at 11:31:53AM +, Stuart Henderson wrote: > On 2018/11/04 10:29, Daniel Dickman wrote: > > The below overrides the cached autoconf value that says that we have > > sbrk(2) on our system and pretends like we don't have it. > > > > With this we can build tcsh without a need for sbrk(2). > > > > ok? > > Should we just remove or change the entry in infrastructure/db/config.site > instead? > > This is where it shows up. > It looks worthwhile to try. Or we could just finally remove brk and sbrk from the libc ? Now that emacs21 is dead, is there any need for those functions ? Archeology shows them in POSIX 1997, marked as "legacy" at that point. I don't see them even mentioned in POSIX 2017
Re: locate.mklocatedb broken with LC_ALL!=C
On Sun, Oct 07, 2018 at 09:43:05AM +0200, Giovanni Bechis wrote: > Hi, > after setting LC_ALL=en_US.UTF-8 on my env locate.mklocatedb seems broken, > resetting LC_ALL=C is a workaround. > > $ export LC_ALL=en_US.UTF-8 > $ doas /usr/libexec/locate.updatedb > sort: Illegal byte sequence > locate.mklocatedb: cannot build locate database > $ export LC_ALL=C > $ doas /usr/libexec/locate.updatedb > > Should we run weekly(8) with LC_ALL=C to be sure that locate.updatedb runs > correctly ? > > Cheers > Giovanni Fixing locate.mklocatedb looks much better. Specifically, the only part that cares about locale is sort, and it's definitely correct in fixing it's not run on an utf-8 file.
pkg_add testing
5 years agom I introduced a tweak to pkg_add to forego updates when they weren't strictly necessary. Then I made it the default. This caused some unidentified problems at the time, and the code was backed out by kili@ soon afterward. I just *fixed* a weird bug in pkg_add, and there's a good chance it's directly related: the bug would show up only when packages said "no need to update" but leading to a shared library a few steps down in the chain of depends; since the older tweak would lead to fewer updates, it would trigger the bug more often. So now is the time for testing that tweak again, see if it can become default. Please test: pkg_add -u -DSHORTENED will forego updates when the package being updated didn't really change version (the update would only be triggered because the packages it was built against were slightly newer, but the shared libraries would be the same). If nothing untowards happens, it probably means we nailed that 5 years old problem...
Re: make build as root fails when SUDO=doas
On Mon, Dec 10, 2018 at 03:37:48PM -0500, Ted Unangst wrote: > Solene Rapenne wrote: > > Not sure how to fix it. Maybe people shouldn't try to compile as root when > > having SUDO=doas set and then, it's not an issue anymore? > > yeah, i would say this is an issue of your own making. you ask to use doas, > make is going to use doas. This is not really acceptable, ports and src should play nice with each other, usually.
Re: make build as root fails when SUDO=doas
On Mon, Dec 10, 2018 at 01:33:49PM +0100, Solene Rapenne wrote:
> hi
>
> I have SUDO=doas in /etc/mk.conf for ports, this is preventing a `make build`
> in /usr/src as root if /etc/doas.conf doesn't have a line "permit nopass root
> as root". This fails when using "doas" in regress/usr/bin/ssh/
>
> doas: Operation not permitted
> *** Error 1 in regress/usr.bin/ssh (Makefile:212 'clean')
> *** Error 1 in regress/usr.bin (:48 'cleandir')
> *** Error 1 in regress (:48 'cleandir')
> *** Error 1 in . (:48 'cleandir')
> *** Error 1 in . (Makefile:86 'do-build')
> *** Error 1 in /usr/src (Makefile:74 'build')
>
>
> the issue comes from the 3rd line of that extract from Makefile:212
>
> clean: ${CLEAN_SUBDIR}
> rm -f ${CLEANFILES}
> test -z "${SUDO}" || ${SUDO} rm -f ${SUDO_CLEAN}
> rm -rf .putty
>
> Not sure how to fix it. Maybe people shouldn't try to compile as root when
> having SUDO=doas set and then, it's not an issue anymore?
There are several possibilities:
- add a test similar to the one in src/Makefile, e.g., not run
sudo if you're root already (relatively complicated for no obvious benefit)
- try to remove the files normally first
rm -f ${SUDO_CLEAN} || test -z "${SUDO}" || ${SUDO} rm -f ${SUDO_CLEAN}
this should actually fix the issue.
Any other directory with that problem ?
Re: make build as root fails when SUDO=doas
On Mon, Dec 10, 2018 at 08:17:04PM +0100, Anton Lindqvist wrote:
> On Mon, Dec 10, 2018 at 01:33:49PM +0100, Solene Rapenne wrote:
> > hi
> >
> > I have SUDO=doas in /etc/mk.conf for ports, this is preventing a `make
> > build`
> > in /usr/src as root if /etc/doas.conf doesn't have a line "permit nopass
> > root
> > as root". This fails when using "doas" in regress/usr/bin/ssh/
> >
> > doas: Operation not permitted
> > *** Error 1 in regress/usr.bin/ssh (Makefile:212 'clean')
> > *** Error 1 in regress/usr.bin (:48 'cleandir')
> > *** Error 1 in regress (:48 'cleandir')
> > *** Error 1 in . (:48 'cleandir')
> > *** Error 1 in . (Makefile:86 'do-build')
> > *** Error 1 in /usr/src (Makefile:74 'build')
> >
> >
> > the issue comes from the 3rd line of that extract from Makefile:212
> >
> > clean: ${CLEAN_SUBDIR}
> > rm -f ${CLEANFILES}
> > test -z "${SUDO}" || ${SUDO} rm -f ${SUDO_CLEAN}
> > rm -rf .putty
> >
> > Not sure how to fix it. Maybe people shouldn't try to compile as root when
> > having SUDO=doas set and then, it's not an issue anymore?
>
> I have the following line in my /etc/mk.conf:
>
> SUDO!!=[ `id -u` -ne 0 ] && echo /usr/bin/doas; true
That's not really a solution. It means you are going to run this
each time you run any bsd makefile.
Re: make build as root fails when SUDO=doas
On Tue, Dec 11, 2018 at 02:35:33PM -0700, Theo de Raadt wrote:
> Ted Unangst wrote:
>
> > Marc Espie wrote:
> > > > > - try to remove the files normally first
> > > > > rm -f ${SUDO_CLEAN} || test -z "${SUDO}" || ${SUDO} rm -f
> > > > > ${SUDO_CLEAN}
> > > > >
> > > > > this should actually fix the issue.
> > > > >
> > > > > Any other directory with that problem ?
> > > >
> > > > that fix the issue and the build continues fine
> > >
> > > So okay from source people ? tedu, guenther, theo, krw ? somebody else ?
> >
> > does anywhere else in the tree do this? aren't most (all) things either done
> > as root or not done as root?
>
> I also don't understand what the point is here.
>
> release(9) shows the correct build process.
>
> you start build as root, to permit the priv-drop security model we
> designed in 2017.
>
> If on the other hand you build from a regular user below, with doas
> configured to allow escalation at any point in time, the regular user
> below CAN ALWAYS BECOME ROOT SO YOU HAVE NO SECURITY MODEL IN MIND AT
> ALL, while you operate on Makefile and such you downloaded from elsewhere
>
> Such use of sudo/doas is an ANTI-PATTERN
Ah, so actually just
rm -f ${SUDO_CLEAN}
should be fine ?
Re: make build as root fails when SUDO=doas
On Tue, Dec 11, 2018 at 06:50:07PM +0100, Solene Rapenne wrote:
> Marc Espie wrote:
> > On Mon, Dec 10, 2018 at 01:33:49PM +0100, Solene Rapenne wrote:
> > > hi
> > >
> > > I have SUDO=doas in /etc/mk.conf for ports, this is preventing a `make
> > > build`
> > > in /usr/src as root if /etc/doas.conf doesn't have a line "permit nopass
> > > root
> > > as root". This fails when using "doas" in regress/usr/bin/ssh/
> > >
> > > doas: Operation not permitted
> > > *** Error 1 in regress/usr.bin/ssh (Makefile:212 'clean')
> > > *** Error 1 in regress/usr.bin (:48 'cleandir')
> > > *** Error 1 in regress (:48 'cleandir')
> > > *** Error 1 in . (:48 'cleandir')
> > > *** Error 1 in . (Makefile:86 'do-build')
> > > *** Error 1 in /usr/src (Makefile:74 'build')
> > >
> > >
> > > the issue comes from the 3rd line of that extract from Makefile:212
> > >
> > > clean: ${CLEAN_SUBDIR}
> > > rm -f ${CLEANFILES}
> > > test -z "${SUDO}" || ${SUDO} rm -f ${SUDO_CLEAN}
> > > rm -rf .putty
> > >
> > > Not sure how to fix it. Maybe people shouldn't try to compile as root when
> > > having SUDO=doas set and then, it's not an issue anymore?
> >
> > There are several possibilities:
> > - add a test similar to the one in src/Makefile, e.g., not run
> > sudo if you're root already (relatively complicated for no obvious benefit)
> >
> > - try to remove the files normally first
> > rm -f ${SUDO_CLEAN} || test -z "${SUDO}" || ${SUDO} rm -f
> > ${SUDO_CLEAN}
> >
> > this should actually fix the issue.
> >
> > Any other directory with that problem ?
>
> that fix the issue and the build continues fine
So okay from source people ? tedu, guenther, theo, krw ? somebody else ?
Re: make build as root fails when SUDO=doas
On Tue, Dec 11, 2018 at 10:55:25PM +0100, Claudio Jeker wrote:
> On Tue, Dec 11, 2018 at 02:35:33PM -0700, Theo de Raadt wrote:
> > Ted Unangst wrote:
> >
> > > Marc Espie wrote:
> > > > > > - try to remove the files normally first
> > > > > > rm -f ${SUDO_CLEAN} || test -z "${SUDO}" || ${SUDO} rm -f
> > > > > > ${SUDO_CLEAN}
> > > > > >
> > > > > > this should actually fix the issue.
> > > > > >
> > > > > > Any other directory with that problem ?
> > > > >
> > > > > that fix the issue and the build continues fine
> > > >
> > > > So okay from source people ? tedu, guenther, theo, krw ? somebody else ?
> > >
> > > does anywhere else in the tree do this? aren't most (all) things either
> > > done
> > > as root or not done as root?
> >
> > I also don't understand what the point is here.
> >
> > release(9) shows the correct build process.
> >
> > you start build as root, to permit the priv-drop security model we
> > designed in 2017.
> >
> > If on the other hand you build from a regular user below, with doas
> > configured to allow escalation at any point in time, the regular user
> > below CAN ALWAYS BECOME ROOT SO YOU HAVE NO SECURITY MODEL IN MIND AT
> > ALL, while you operate on Makefile and such you downloaded from elsewhere
> >
> > Such use of sudo/doas is an ANTI-PATTERN
>
> I think the main issue is that /usr/sr/regress was not moved to the
> priv-drop security model. There is bunch of code which needs root but I
> don't want to run all of regress as user root.
There is a kind of mixed model there.
Because make build still goes thru regress for obj and cleandir
Yet the rest of the build doesn't!
So, if we agree that it needs to stay the way it currently is, then
the SUDO in that Makefile might trigger while running as root...
... or we could change all the ports tree and rename SUDO to something
else in there so that it doesn't interfere at all. But I see most porters
not too happy with that choice.
As Mr Morden would say "what do you want ?"...
Re: pkg_add testing
On Wed, Dec 12, 2018 at 03:20:49PM +0100, Marc Espie wrote: > Please test: > > pkg_add -u -DSHORTENED And Landry found an issue. I forgot to update some code when I tweaked stuff to add @version support. Please try again with just committed pkg_add code... (basically, the older version didn't update some stuff it should have, so you'll just have slightly more updates than before, shouldn't be harmful
Re: make build as root fails when SUDO=doas
On Wed, Dec 12, 2018 at 11:12:17AM +0100, Alexander Bluhm wrote:
> On Tue, Dec 11, 2018 at 11:00:30PM +0100, Marc Espie wrote:
> > Ah, so actually just
> > rm -f ${SUDO_CLEAN}
> >
> > should be fine ?
>
> Regress jumps from root to non-root in a very inconsistent way. It
> could be improved, but that would be a lot of work. The result
> will not be perfect as tests have very different needs for permissions.
> So I would recommend to leave it as it is unless someone volunteers
> to refactor all the tests.
>
> If a test creates a file as root with an ugly ${SUDO} hack, it
> should use the same hack to clean it. Then the test is self
> contained. Care should be taken that the make clean hack does not
> affect the make build.
>
> So I like espie@'s suggestion to try rm first without ${SUDO} for
> people running make build, and then with ${SUDO} for people who
> have left overs from testing.
>
> bluhm
Well, apart from the bike-shedding, it seems like the most correct
short-term solution.
So I will commit it tomorrow, unless someone has an actual better idea.
Re: make build as root fails when SUDO=doas
On Fri, Dec 14, 2018 at 03:57:12PM -0500, Ted Unangst wrote:
> Marc Espie wrote:
> > Well, apart from the bike-shedding, it seems like the most correct
> > short-term solution.
> >
> > So I will commit it tomorrow, unless someone has an actual better idea.
>
> Nobody answered if SUDO_CLEAN is actualy set. I checked. It's not.
>
> Index: Makefile
> ===
> RCS file: /cvs/src/regress/usr.bin/ssh/Makefile,v
> retrieving revision 1.98
> diff -u -p -r1.98 Makefile
> --- Makefile 22 Nov 2018 08:48:32 - 1.98
> +++ Makefile 14 Dec 2018 20:56:11 -
> @@ -209,7 +209,6 @@ c-${s}:
>
> clean: ${CLEAN_SUBDIR}
> rm -f ${CLEANFILES}
> - test -z "${SUDO}" || ${SUDO} rm -f ${SUDO_CLEAN}
> rm -rf .putty
>
> .include
Oh, I had a slightly older version.
So, yep, it was just dtucker@ half-committing some clean-up.
Okay to finish fixing it. :)
Re: strlcpy() or strscpy()?
On Sun, Jan 27, 2019 at 01:07:05AM -0500, 0sjfoij...@firemail.cc wrote:
> Recently on LCA2019, Joel Sing made a presentation about "Security
> Vulnerability Mitigations"[1]
> (very good, btw). He suggests function strlcpy(3) as a secure API.
> In the same conference, though, Kees Cook ("Making C Less Dangerous in the
> Linux kernel"[2]),
> recommends strscpy() as more secure. So, my question is: what's the best to
> use?
>
If anything, strscpy() made things more INsecure, by adding another
confusing variation on a well-defined API that has proven itself.
strscpy reeks of "Not Invented Here", "Misplaced Hubris" and "Design by
committee" syndrome, which happens so very often in Linux-land.
Their only saving grace is darwinian: there are so many lemmings over there
that ludicrous ideas tend to get replaced by better ideas over time.
Re: remove date from signify zsig
On Mon, Feb 25, 2019 at 03:02:42PM -0500, Ted Unangst wrote: > Andre Stoebe wrote: > > Hi, > > > > I, too, would like to have a way of signing the gzip archive in a > > reproducible way, so here's a diff that uses -n, similar to gzip(1). > > > > However, if that's a bad idea, I'm fine with continuing to use an > > unsigned gzip archive and creating a sigfile with signify. > > Let me think on this for a bit. Seems reasonable, though. If you want something simpler, just set the date from outside through an env variable, so you'll have a reproducible date line for when you absolutely need it.
Re: Harmonize examples in signify man page
On Sun, Feb 24, 2019 at 10:09:43PM +0100, Matthias Schmidt wrote: > Hi, > > while looking at signify's man page I noticed that the names of the > arguments in the EXAMPLES section change (newkey.sec vs key.sec, etc). > The diff harmonizes the examples. It makes it easier to follow the examples. > > Cheers > > Matthias > > diff --git signify.1 signify.1 > index de3b433e3b0..9f7335352a2 100644 > --- signify.1 > +++ signify.1 > @@ -153,13 +153,13 @@ The message file is too large. > .El > .Sh EXAMPLES > Create a new key pair: > -.Dl $ signify -G -p newkey.pub -s newkey.sec > +.Dl $ signify -G -p key.pub -s key.sec > .Pp > Sign a file, specifying a signature name: > -.Dl $ signify -S -s key.sec -m message.txt -x msg.sig > +.Dl $ signify -S -s key.sec -m message -x message.sig Your mileage may vary, but this makes things harder to follow for me. Obviously visually differentiating between creating new keys and using existing ones makes a lot of sense for me. Especially since the examples are not meant to be chained together, at all. This is a reference manual, not a tutorial.
Re: remove date from signify zsig
On Mon, Feb 25, 2019 at 05:11:54PM -0500, Ted Unangst wrote: > Marc Espie wrote: > > On Mon, Feb 25, 2019 at 03:02:42PM -0500, Ted Unangst wrote: > > > Andre Stoebe wrote: > > > > Hi, > > > > > > > > I, too, would like to have a way of signing the gzip archive in a > > > > reproducible way, so here's a diff that uses -n, similar to gzip(1). > > > > > > > > However, if that's a bad idea, I'm fine with continuing to use an > > > > unsigned gzip archive and creating a sigfile with signify. > > > > > > Let me think on this for a bit. Seems reasonable, though. > > > > If you want something simpler, just set the date from outside through an > > env variable, so you'll have a reproducible date line for when you > > absolutely > > need it. > > Like TZ? I don't think there's a way to change the time that way. Is there? No, but instead of an extra option, a specific env variable ? might make more sense... or less. I don't know. I'm surprised this surfaced again, as the subject was broached a few months ago and dismissed, because yep, we do want the timestamp to mean something for pkg_add. Especially relating to our keys having a shelf life.
Re: [patch] improve strptime(3) %z timezone parsing
On Sun, Mar 17, 2019 at 03:10:04PM -0400, Ted Unangst wrote: > Hiltjo Posthuma wrote: > > Not all BSD man pages reference the same RFCs, like RFC5322. The OpenBSD > > strptime(3) man page (%z) describes military/nautical zones are supported. > > > > - NetBSD supports it (correctly since only a few months ago and it is not in > > stable or backported). > > - FreeBSD and DragonFlyBSD do not support them (strptime returns NULL). > > - Linux glibc does not support them (strptime returns NULL). > > - Linux musl libc does not support %z. > > This sounds like we should just drop support as well. I think that's better > than supporting something that's clearly unreliable. By that reckoning, you would never implement anything new... :(
Re: install(1) broken
On Sat, Feb 09, 2019 at 05:23:09PM -0500, Ted Unangst wrote: > Marc Espie wrote: > > hey, your commit to install(1) broke something. > > > > Specifically lang/go-boostrap now produces a broken package which can't > > be used to build go. > > > > All the go/bootstrap/pkg/tool/openbsd_amd64/* > > have lost their x bit > > > > Relevant fake install information, it definitely looks like the last line > > is now a no-op. > > > # These get installed via `find' however we need them to be executable > > /pobj/go-bootstrap-1.4.20171003/bin/install -d -m 755 > > /pobj/go-bootstrap-1.4.20171003/fake-amd64/usr/local/go/bootstrap/pkg/tool//openbsd_amd64 > > /pobj/go-bootstrap-1.4.20171003/bin/install -c -m 755 -p > > /pobj/go-bootstrap-1.4.20171003/go/pkg/tool//openbsd_amd64/* > > /pobj/go-bootstrap-1.4.20171003/fake-amd64/usr/local/go/bootstrap/pkg/tool//openbsd_amd64 > > Yes. This is a weird way to invoke chmod, but that's what it wants. This actually makes some kind of sense in a broader context. Specifically, install_flags are a somewhat standard way to enforce ownership/permissions on a file, whether while copying it from one place to another... or after moving it. It's way simpler to bundle everything into a single variable than having several separate variables to do things.
Re: firmware.openbsd.org (SHA256)
On Wed, Feb 13, 2019 at 04:41:56PM +0100, Oleg Pahl wrote: > Hi all, > > I use 6.4 Release. > I install fm on my laptop from http://firmware.openbsd.org/firmware/6.4/ > This URL i found in man page FW_UPDATE(1) > You can see that ( index.txt ) has one file more then as on server! It doesn't matter. Getting a consistent global SHA256 / SHA256.sig for distributed sets of packages or firmwares is difficult at best. For precisely that reason, packages are individually signed. And both pkg_add *and* fw_update will refuse to install anything that's not signed *by default*. You can actually check the signature yourself, it's directly in the gzip header comment (so that you can't pass unsigned data through zlib for decompressions). RTFM signify(1) -z mode
Re: install(1) broken
On Sun, Feb 10, 2019 at 03:55:41PM +0100, Marc Espie wrote:
> Something similar to this perhaps ?
> Not fully tested yet, but it should avoid the race of trying to
> unlink tempfile several times, and also fix the file name in error messages.
That's now been tested (gone thru a build + xbuild and bulk in progress
that went thru go-bootstrap and go without issues).
Okay ?
This needs re-applying Ingo's patch first, of course, which I can either do
or we coordinate with Ingo.
Index: xinstall.c
===
RCS file: /cvs/src/usr.bin/xinstall/xinstall.c,v
retrieving revision 1.68
diff -u -p -r1.68 xinstall.c
--- xinstall.c 8 Feb 2019 12:53:44 - 1.68
+++ xinstall.c 10 Feb 2019 14:53:49 -
@@ -222,6 +222,7 @@ install(char *from_name, char *to_name,
struct timespec ts[2];
int devnull, from_fd, to_fd, serrno, files_match = 0;
char *p;
+ char *target_name = tempfile;
(void)memset((void *)_sb, 0, sizeof(from_sb));
(void)memset((void *)_sb, 0, sizeof(to_sb));
@@ -311,10 +312,14 @@ install(char *from_name, char *to_name,
} else {
files_match = 1;
(void)unlink(tempfile);
+ target_name = to_name;
+ (void)close(temp_fd);
}
}
- (void)close(to_fd);
- to_fd = temp_fd;
+ if (!files_match) {
+ (void)close(to_fd);
+ to_fd = temp_fd;
+ }
}
/*
@@ -333,13 +338,15 @@ install(char *from_name, char *to_name,
if ((gid != (gid_t)-1 || uid != (uid_t)-1) &&
fchown(to_fd, uid, gid)) {
serrno = errno;
- (void)unlink(tempfile);
- errx(1, "%s: chown/chgrp: %s", tempfile, strerror(serrno));
+ if (target_name == tempfile)
+ (void)unlink(tempfile);
+ errx(1, "%s: chown/chgrp: %s", target_name, strerror(serrno));
}
if (fchmod(to_fd, mode)) {
serrno = errno;
- (void)unlink(tempfile);
- errx(1, "%s: chmod: %s", tempfile, strerror(serrno));
+ if (target_name == tempfile)
+ (void)unlink(tempfile);
+ errx(1, "%s: chmod: %s", target_name, strerror(serrno));
}
/*
@@ -349,7 +356,7 @@ install(char *from_name, char *to_name,
if (fchflags(to_fd,
flags & SETFLAGS ? fset : from_sb.st_flags & ~UF_NODUMP)) {
if (errno != EOPNOTSUPP || (from_sb.st_flags & ~UF_NODUMP) != 0)
- warnx("%s: chflags: %s", tempfile, strerror(errno));
+ warnx("%s: chflags: %s", target_name, strerror(errno));
}
if (flags & USEFSYNC)
Re: install(1) broken
Something similar to this perhaps ?
Not fully tested yet, but it should avoid the race of trying to
unlink tempfile several times, and also fix the file name in error messages.
Index: xinstall.c
===
RCS file: /cvs/src/usr.bin/xinstall/xinstall.c,v
retrieving revision 1.68
diff -u -p -r1.68 xinstall.c
--- xinstall.c 8 Feb 2019 12:53:44 - 1.68
+++ xinstall.c 10 Feb 2019 14:53:49 -
@@ -222,6 +222,7 @@ install(char *from_name, char *to_name,
struct timespec ts[2];
int devnull, from_fd, to_fd, serrno, files_match = 0;
char *p;
+ char *target_name = tempfile;
(void)memset((void *)_sb, 0, sizeof(from_sb));
(void)memset((void *)_sb, 0, sizeof(to_sb));
@@ -311,10 +312,14 @@ install(char *from_name, char *to_name,
} else {
files_match = 1;
(void)unlink(tempfile);
+ target_name = to_name;
+ (void)close(temp_fd);
}
}
- (void)close(to_fd);
- to_fd = temp_fd;
+ if (!files_match) {
+ (void)close(to_fd);
+ to_fd = temp_fd;
+ }
}
/*
@@ -333,13 +338,15 @@ install(char *from_name, char *to_name,
if ((gid != (gid_t)-1 || uid != (uid_t)-1) &&
fchown(to_fd, uid, gid)) {
serrno = errno;
- (void)unlink(tempfile);
- errx(1, "%s: chown/chgrp: %s", tempfile, strerror(serrno));
+ if (target_name == tempfile)
+ (void)unlink(tempfile);
+ errx(1, "%s: chown/chgrp: %s", target_name, strerror(serrno));
}
if (fchmod(to_fd, mode)) {
serrno = errno;
- (void)unlink(tempfile);
- errx(1, "%s: chmod: %s", tempfile, strerror(serrno));
+ if (target_name == tempfile)
+ (void)unlink(tempfile);
+ errx(1, "%s: chmod: %s", target_name, strerror(serrno));
}
/*
@@ -349,7 +356,7 @@ install(char *from_name, char *to_name,
if (fchflags(to_fd,
flags & SETFLAGS ? fset : from_sb.st_flags & ~UF_NODUMP)) {
if (errno != EOPNOTSUPP || (from_sb.st_flags & ~UF_NODUMP) != 0)
- warnx("%s: chflags: %s", tempfile, strerror(errno));
+ warnx("%s: chflags: %s", target_name, strerror(errno));
}
if (flags & USEFSYNC)
Re: install(1) broken
On Sat, Feb 09, 2019 at 05:23:09PM -0500, Ted Unangst wrote:
> Marc Espie wrote:
> > hey, your commit to install(1) broke something.
> >
> > Specifically lang/go-boostrap now produces a broken package which can't
> > be used to build go.
> >
> > All the go/bootstrap/pkg/tool/openbsd_amd64/*
> > have lost their x bit
> >
> > Relevant fake install information, it definitely looks like the last line
> > is now a no-op.
>
> > # These get installed via `find' however we need them to be executable
> > /pobj/go-bootstrap-1.4.20171003/bin/install -d -m 755
> > /pobj/go-bootstrap-1.4.20171003/fake-amd64/usr/local/go/bootstrap/pkg/tool//openbsd_amd64
> > /pobj/go-bootstrap-1.4.20171003/bin/install -c -m 755 -p
> > /pobj/go-bootstrap-1.4.20171003/go/pkg/tool//openbsd_amd64/*
> > /pobj/go-bootstrap-1.4.20171003/fake-amd64/usr/local/go/bootstrap/pkg/tool//openbsd_amd64
>
> Yes. This is a weird way to invoke chmod, but that's what it wants.
>
> I think this was a long standing bug in install? If the files match, we need
> to continue on with to_fd == existing file so that metadata updates work.
>
>
> Index: xinstall.c
> ===
> RCS file: /cvs/src/usr.bin/xinstall/xinstall.c,v
> retrieving revision 1.68
> diff -u -p -r1.68 xinstall.c
> --- xinstall.c8 Feb 2019 12:53:44 - 1.68
> +++ xinstall.c9 Feb 2019 22:21:03 -
> @@ -313,8 +313,12 @@ install(char *from_name, char *to_name,
> (void)unlink(tempfile);
> }
> }
> - (void)close(to_fd);
> - to_fd = temp_fd;
> + if (!files_match) {
> + (void)close(to_fd);
> + to_fd = temp_fd;
> + } else {
> + close(temp_fd);
> + }
> }
>
> /*
I'm afraid this needs to be slightly more complex, probably to put the
remaining code in its own function, or something.
Specifically, the error paths for the fchown and fchmod will be all bogus
in that case, referring to a tempfile that's already been removed, and
is not even the target...
Re: PKG_SIGN(1) - D argument
On Fri, Feb 15, 2019 at 11:55:59AM +0100, Oleg Pahl wrote: > Hi @all, > > Is it ok, that on man page *PKG_SIGN(1)* there is*[-D name=[value]]* > argument, > > but there is no info about value of arg. below ... (I mean what kind of > value's is possible to use with -D) > > I think, that its will be better to put this Info on man page. Not just for > me =) There are options intentionally left undocumented in a lot of places. Sometimes they are not intended for public consumption for instance (non regress test comes to mind) but they're part of the command syntax anyway. Have fun finding all the places where this happens, pkg_sign(1) is definitely not the only command with undocumented options.
Re: locate.mklocatedb broken with LC_ALL!=C
On Fri, Feb 15, 2019 at 07:58:48PM +0100, Giovanni Bechis wrote:
> ping...
> any possible issue with millert@ diff ?
> Giovanni
>
> On Sun, Oct 07, 2018 at 08:35:28PM -0600, Todd C. Miller wrote:
> > On Sun, 07 Oct 2018 17:08:06 +0200, Marc Espie wrote:
> >
> > > Specifically, the only part that cares about
> > > locale is sort, and it's definitely correct in fixing
> > > it's not run on an utf-8 file.
> >
> > Agreed. How about the following?
> >
> > - todd
> >
> > Index: usr.bin/locate/locate/mklocatedb.sh
> > ===
> > RCS file: /cvs/src/usr.bin/locate/locate/mklocatedb.sh,v
> > retrieving revision 1.13
> > diff -u -p -u -r1.13 mklocatedb.sh
> > --- usr.bin/locate/locate/mklocatedb.sh 18 Mar 2007 20:13:49 -
> > 1.13
> > +++ usr.bin/locate/locate/mklocatedb.sh 8 Oct 2018 02:34:52 -
> > @@ -66,7 +66,8 @@ filelist=`mktemp ${TMPDIR=/tmp}/_filelis
> > }
> > trap 'rm -f $bigrams $filelist' 0 1 2 3 5 10 15
> >
> > -if $sortcmd $sortopt > $filelist; then
> > +# Run sort in the C locale or binary data may be interpreted as UTF-8
> > +if LC_ALL=C $sortcmd $sortopt > $filelist; then
> > $bigram < $filelist | $sort -nr |
> > awk -Ft 'BEGIN { ORS = "" } NR <= 128 { print $2 }' >
> > $bigrams &&
> > $code $bigrams < $filelist
Oh, I thought it had been committed ages ago
Re: friendlier doas persist
On Wed, Jan 23, 2019 at 05:55:54PM -0500, Ted Unangst wrote: > Stuart Henderson wrote: > > On 2019/01/22 21:46, Ted Unangst wrote: > > > The persist feature in doas (actually the kernel side implementation) has > > > some > > > additional checks. The idea was to prevent accidental usage, but in > > > practice > > > it seems this is making life more difficult than necessary. It's cost > > > without > > > benefit. This diff relaxes the session checks so it should be possible to > > > use > > > doas in ports building, etc. > > > > Fixing this would be very welcome and would stop some ports users from > > either running with nopass or continuing to use sudo. But it doesn't help: > > > > configure doas to use persist > > set SUDO=doas in mk.conf > > cd /usr/ports/math/moo# example simple/quick port > > make reinstall > > > > Here you're asked for your password twice. > > Is make creating a second process group? I guess it is for job control? Need > to find a place to stick the verauth flag such that later doas can find it... No it's not. Doesn't mean *something else* in the chain isn't.
Re: Update perl to 5.28.1
On Sun, Feb 03, 2019 at 02:23:16PM +, Stuart Henderson wrote:
> On 2018/12/24 19:05, Andrew Hewus Fresh wrote:
> > I've finally gotten our local patches applied to perl 5.28.1 and it now
> > could use some testing. I was trying to get a few more of my
> > architectures working, but I ran out of time and now I have to go visit
> > family for the holidays so it's up to y'all to do some testing for me.
> >
> >
> > The infrastructure to do that is still here on github, including
> > instructions on using it:
> >
> > https://github.com/afresh1/OpenBSD-perl/
> >
> >
> >
> > You can also download a pre-patched perl from here, use it to replace
> > src/gnu/usr.bin/perl and do a normal build to get it installed, do note
> > that any XS modules that were built with the old version of perl will no
> > longer work, so be careful.
> >
> > http://cvs.afresh1.com/~andrew/OpenBSD-perl-5.28.1.tar.gz
> >
> >
> > You can read more on the new features and changes on the metacpan.
> >
> > https://metacpan.org/release/SHAY/perl-5.28.1
> >
> > You find them in the perldelta for the versions since 5.24.
> >
> > https://metacpan.org/pod/release/SHAY/perl-5.28.1/pod/perl5260delta.pod
> > https://metacpan.org/pod/release/SHAY/perl-5.28.1/pod/perl5280delta.pod
> > https://metacpan.org/pod/release/SHAY/perl-5.28.1/pod/perldelta.pod
> >
> >
> > One of the important changes, and the one most likely to cause trouble
> > is the removal of "." from the @INC library search list.
>
> Having fixed a bunch of these, and cwen@ fixing a bunch more, and still
> having 100-odd ports failing to build, I have come round to the idea of
> setting PERL_USE_UNSAFE_INC instead. I wasn't super happy about this but
> the upstream cpan tool seems resigned to doing this for now so we might
> as well follow suit.
>
> It seems sanest to set this in cpan.port.mk rather than individual Makefiles:
>
> Index: cpan.port.mk
> ===
> RCS file: /cvs/ports/infrastructure/mk/cpan.port.mk,v
> retrieving revision 1.20
> diff -u -p -r1.20 cpan.port.mk
> --- cpan.port.mk 20 Mar 2016 19:56:44 - 1.20
> +++ cpan.port.mk 3 Feb 2019 14:10:27 -
> @@ -24,6 +24,11 @@ TEST_DEPENDS +=devel/p5-Test-Pod \
> devel/p5-Test-Pod-Coverage
> .endif
>
> +# perl 5.26+ no longer has "." in @INC by default, but it's widely required
> in
> +# build/test systems. set it locally, as is also done in the upstream cpan
> tool.
> +CONFIGURE_ENV += PERL_USE_UNSAFE_INC=1
> +TEST_ENV += PERL_USE_UNSAFE_INC=1
> +
> MODCPAN_POST_INSTALL = ${INSTALL_DATA_DIR} ${MODCPAN_EXAMPLES_DIR}; \
> cd ${WRKSRC}/${MODCPAN_EXAMPLES_DIST}/ && pax -rw .
> ${MODCPAN_EXAMPLES_DIR};\
> chown -R ${SHAREOWN}:${SHAREGRP} ${MODCPAN_EXAMPLES_DIR}
>
>
> Applying that diff fixes 80+. Any major concerns about doing that
> (especially espie@, afresh1@)? It hasn't been through a full build
> (I added it after a run to mop up breakage) but I can do that.
No, I think it's sane to do that. Let cpan mop up most of the breakage,
then we'll remove that.
Re: make.1: add missing dependency to example
On Wed, Jan 30, 2019 at 01:56:04PM -0600, Scott Cheloha wrote:
> We need to tell ${CC} about b.o explicitly.
>
> ok?
>
> -Scott
>
> P.S. How was BSD $> or GNU $^ or an equivalent *not* standardized as
> an automatic variable? I get that it isn't a silver bullet, but in
> the relatively common "let's take several and make a " case it
> is extremely useful and totally missing from the portable syntax.
Well, it's not standardized.
As far as adding extensions to bsd tools, it's always a bit annoying,
because we then foster making things *less* portable to other environments
that don't have it. So generally, it's more a question of knowing when
it's wide-spread enough (e.g., sed -i).
make is *very* sneaky in that regard... :(
In general, the portable way to do things properly is that as soon as you
have two objects files, you got thru a variable, which is good practice
anyway.
> Index: make.1
> ===
> RCS file: /cvs/src/usr.bin/make/make.1,v
> retrieving revision 1.127
> diff -u -p -r1.127 make.1
> --- make.14 Jul 2018 14:11:49 - 1.127
> +++ make.130 Jan 2019 19:45:15 -
> @@ -464,7 +464,7 @@ from sources a.c and b.c, with header fi
> ${CC} ${CFLAGS} -c $<
>
> prog: a.o b.o
> - ${CC} ${CFLAGS} -o $@ a.o
> + ${CC} ${CFLAGS} -o $@ a.o b.o
>
> a.o b.o: a.h
>
Yep, okay
chroot vs su vs doas
So, in dpb, I've been forking a lot of 'chroot -u user /build'
to build various things, and it works just great.
I was wondering about the benefits of
su ${BUILDUSER} -c 'some quoted command'
vs
chroot -u ${BUILDUSER} / some unquoted command
Superficially, it looks mostly similar.
The very nice thing about chroot (IMO) being that you can pass the
command verbatim without having to re-quote anything. The other
difference being that chroot doesn't fork an extra shell, which
might make things more transparent wrt running commands.
I'm also wondering about doas.
By default, it's not configured at all.
But what would it hurt to allow root usage ?
Specifically,
doas -u ${BUILDUSER} some unquoted command
as run by root. This would not open any security hole, would it ?
Finally, I'm wondering if people would have any use for a chroot'd
option in doas, and whether it's a security issue (again).
Like, people have some hardened doas.conf which only allows running
some commands as root.
Some of these commands are basically game over, as they allow anything
to be run, actually. Specifically, /usr/bin/env, or chroot...
Would
doas -c /rootdir somecmd
be of any use ?
Re: New implementation and interface for strlcpy and strlcat
On Fri, May 24, 2019 at 07:22:07PM +0700, Oleg Chumanov wrote:
> First of all, I really want to thank you for your files.
>
> However, Can you show me an example of these:
>
> > makes it easier to perform error
> > recovery in the caller. For example, the caller may wish to
> > reallocate the buffer and retry.
> >
> > That would make it a bit harder to do error recovery since you'd
> > need to compute the required length
>
> I mean, I can do something like this:
>
> offset = 0;
> while ((n = olegcpy(buf + offset, str + offset, buf_size - offset))) {
> offset = buf_size - 1;
> buf_size += n;
> buf = realloc(buf, buf_size);
> }
Of course, you could, but you are totally missing the point of
strlcpy/strlcat.
Look, the basic idea is that developers make mistakes.
All of them.
Including *you*.
Not all the time, but once is enough, especially where buffer overflows
are concerned.
Your proposed interface (which I renamed olegcpy) is more complex to use
than regular strlcpy.
Look, you *are* doing pointer arithmetic in your loop.
Oh, btw, your code is wrong... you forgot to check that buf_size didn't
overflow (unlikely, I know), and you *also* forgot to check that realloc
did work.
That's not your point ?
Actually, it *is*. You had to devote brain cells to thinking about your
very smart loop, so you missed a few trivial points.
That's what strlcpy is all about. It's a PLAIN SIMPLE interface designed
for ACTUAL PEOPLE.
Compare that with strlcpy
while (1) {
size_t n = strlcpy(buf, str, buf_size);
if (n < bufsize)
break;
char *newb = realloc(buf, n);
if (!newb) {
free(buf);
/* oh hey, that's an error */
exit(1);
}
buf = newb;
buf_size = n;
}
Oh, hey, I didn't forget to test for actual errors. Because the code
is simpler.
So, yes, I copy the same string twice, once in a blue moon.
Guess what ? FOR REAL PROGRAMS IT DOESN'T MATTER.
*IF* your buffer sizes are carefully chosen, you won't reallocate them
THAT often (especially because the smart thing to do is to devote energy
to actually track average buffer sizes and reuse buffers so you don't
reallocate all the time).
So, in reality, I won't ever realloc *exactly* what strlcpy gives me.
My code looks more like this usually:
while (1) {
size_t n = strlcpy(buf, str, buf_size);
if (n < bufsize)
break;
while (n > bufsize)
bufsize <<= 1U;
char *newb = realloc(buf, bufsize);
if (!newb) {
free(buf);
/* oh hey, that's an error */
exit(1);
}
buf = newb;
}
and I do *love* strlcpy and strlcat because they're simple enough to use
that I can spend my time concentrating on stuff that actually matters,
without too much risk at introducing more bugs in existing programs.
"Premature optimization is the root of all evil" (Don E. Knuth)
Re: Qt5's libtool link scripts are unusable
On Thu, Jun 20, 2019 at 09:41:23PM +0200, Rafael Sadowski wrote: > This's what Qt 5.13 doas. They use "<< endl;" instead of "<< "\n";", I > would like to prefer that. std::endl calls std::flush which synchronizes > with the underlying storage device. > > RS I agree with using endl if that's what Qt 5.13 does. But it's one specific case where you really do not give a fuck about flushing. After all you are generating a Makefile. Who cares about synchronizing after each single line ?
Re: Remove duplicate pledge(2) from tsort(1)
On Thu, Jul 11, 2019 at 01:20:11PM +0100, Ricardo Mestre wrote:
> Hi,
>
> pledge "stdio rpath" is already called in main(), so we can remove the
> duplicate from parse_args(), along with the pledge commented out from another
> era!
>
> The second part is about placing pledge "stdio" in main() instead for better
> readability (at least for me).
>
> No functional change is intended here and regress still pass, comments ok?
Sure, I did notice it a few days ago, but did not have time to fix it.
Thanks
> Index: tsort.c
> ===
> RCS file: /cvs/src/usr.bin/tsort/tsort.c,v
> retrieving revision 1.36
> diff -u -p -u -r1.36 tsort.c
> --- tsort.c 20 May 2017 09:31:19 - 1.36
> +++ tsort.c 11 Jul 2019 12:13:00 -
> @@ -879,10 +879,6 @@ parse_args(int argc, char *argv[], struc
>
> files[i] = NULL;
>
> -/* if (pledge("stdio rpath", files) == -1) */
> - if (pledge("stdio rpath", NULL) == -1)
> - err(1, "pledge");
> -
> nodes_init(pairs);
> order = 0;
>
> @@ -910,9 +906,6 @@ parse_args(int argc, char *argv[], struc
> order = read_pairs(stdin, pairs, reverse_flag, "stdin",
> order, hints_flag == 2);
> }
> -
> - if (pledge("stdio", NULL) == -1)
> - err(1, "pledge");
> }
>
> static int
> @@ -1003,6 +996,10 @@ main(int argc, char *argv[])
> err(1, "pledge");
>
> parse_args(argc, argv, );
> +
> + if (pledge("stdio", NULL) == -1)
> + err(1, "pledge");
> +
> return tsort();
> }
>
make patch: let MAKEOBJDIR be more powerful
After some musings, I realized I just had to reorder a few things to
make MAKEOBJDIR way more powerful (and possibly useful)
The idea here is to init vars early, which is easy, and to set up
.CURDIR, MACHINE, MACHINE_ARCH, MACHINE_CPU, so that
MAKEOBJDIR can actually become a full-blown make expression.
e.g., with this something like
MAKEOBJDIR='${.CURDIR}:S/src/obj/}'
will work.
any takers ?
Index: init.c
===
RCS file: /cvs/src/usr.bin/make/init.c,v
retrieving revision 1.7
diff -u -p -r1.7 init.c
--- init.c 2 Oct 2012 10:29:30 - 1.7
+++ init.c 29 Aug 2019 11:01:19 -
@@ -48,8 +48,6 @@ Init(void)
* can be processed correctly */
Parse_Init(); /* Need to initialize the paths of #include
* directories */
- Var_Init(); /* As well as the lists of variables for
-* parsing arguments */
Arch_Init();
Suff_Init();
}
Index: main.c
===
RCS file: /cvs/src/usr.bin/make/main.c,v
retrieving revision 1.123
diff -u -p -r1.123 main.c
--- main.c 22 Apr 2019 18:32:09 - 1.123
+++ main.c 29 Aug 2019 11:01:19 -
@@ -558,12 +558,16 @@ setup_CURDIR_OBJDIR(struct dirs *d)
* and modify the paths for the Makefiles appropriately. The
* current directory is also placed as a variable for make scripts.
*/
- if ((path = getenv("MAKEOBJDIR")) == NULL) {
+ Var_Set(".CURDIR", d->current);
+ if ((path = getenv("MAKEOBJDIR")) == NULL)
path = _PATH_OBJDIR;
- }
+ /* if there's a $ in there, allow substitution */
+ else if (strchr(path, '$'))
+ path = Var_Subst(path, NULL, false);
d->object = chdir_verify_path(path, d);
if (d->object == NULL)
d->object = d->current;
+ Var_Set(".OBJDIR", d->object);
}
/*
@@ -656,6 +660,11 @@ main(int argc, char **argv)
bool read_depend = true;/* false if we don't want to read .depend */
MainParseChdir(argc, argv);
+ Var_Init(); /* do this early for OBJDIR */
+ Var_Set("MACHINE", machine);
+ Var_Set("MACHINE_ARCH", machine_arch);
+ Var_Set("MACHINE_CPU", machine_cpu);
+
setup_CURDIR_OBJDIR();
esetenv("PWD", d.object);
@@ -687,8 +696,6 @@ main(int argc, char **argv)
if (d.object != d.current)
Dir_AddDir(defaultPath, d.current);
- Var_Set(".CURDIR", d.current);
- Var_Set(".OBJDIR", d.object);
Parse_setcurdir(d.current);
Targ_setdirs(d.current, d.object);
@@ -702,9 +709,6 @@ main(int argc, char **argv)
Var_Set(".MAKE", argv[0]);
Var_Set(MAKEFLAGS, "");
Var_Set("MFLAGS", "");
- Var_Set("MACHINE", machine);
- Var_Set("MACHINE_ARCH", machine_arch);
- Var_Set("MACHINE_CPU", machine_cpu);
/*
* First snag any flags out of the MAKEFLAGS environment variable.
synch make doc/defines with reality
This happened a long time ago... okay ? Index: make.1 === RCS file: /cvs/src/usr.bin/make/make.1,v retrieving revision 1.128 diff -u -p -r1.128 make.1 --- make.1 31 Jan 2019 10:27:28 - 1.128 +++ make.1 22 Aug 2019 16:37:14 - @@ -1495,8 +1495,7 @@ uses the following environment variables .Ev MACHINE_ARCH , .Ev MACHINE_CPU , .Ev MAKEFLAGS , -.Ev MAKEOBJDIR , -.Ev MAKEOBJDIRPREFIX , +.Ev MAKEOBJDIR and .Ev PWD . .Nm @@ -1516,10 +1515,6 @@ does not exist system makefile .It Pa /usr/share/mk system makefile directory -.It Pa /usr/obj -default -.Ev MAKEOBJDIRPREFIX -directory .El .Sh EXIT STATUS If Index: pathnames.h === RCS file: /cvs/src/usr.bin/make/pathnames.h,v retrieving revision 1.12 diff -u -p -r1.12 pathnames.h --- pathnames.h 19 Jul 2010 19:46:44 - 1.12 +++ pathnames.h 22 Aug 2019 16:37:14 - @@ -32,18 +32,10 @@ * from: @(#)pathnames.h 5.2 (Berkeley) 6/1/90 */ -#ifdef HAS_PATH_H -# include -#endif -#ifndef _PATH_BSHELL -# define _PATH_BSHELL "/bin/sh" -#endif +#include #ifndef _PATH_OBJDIR #define _PATH_OBJDIR "obj" #endif /* !_PATH_OBJDIR */ -#ifndef _PATH_OBJDIRPREFIX -#define _PATH_OBJDIRPREFIX "/usr/obj" -#endif /* !_PATH_OBJDIRPREFIX */ #ifndef _PATH_DEFSHELLDIR #define _PATH_DEFSHELLDIR "/bin" #endif /* !_PATH_DEFSHELLDIR */
Re: assert() missing in flex
On Mon, Sep 16, 2019 at 05:07:21PM +0800, Michael Mikonos wrote:
> Hello,
>
> When building flex on clang 8.0.1 (i386) I noticed that assert()
> expands to nothing. This happens because of a fallback declaration
> of assert() in flexdef.h when HAVE_ASSERT_H is not set. Instead of
> changing flexdef.h the following patch follows the existing pattern
> in Makefile of adding -DHAVE_SOMETHING. Does this look OK?
>
> - Michael
>
>
> Index: Makefile
> ===
> RCS file: /cvs/src/usr.bin/lex/Makefile,v
> retrieving revision 1.17
> diff -u -p -u -r1.17 Makefile
> --- Makefile 30 Apr 2017 20:30:39 - 1.17
> +++ Makefile 16 Sep 2019 08:55:42 -
> @@ -12,7 +12,7 @@
> # To bootstrap lex, cp initscan.c to scan.c and run make.
>
> PROG= lex
> -CFLAGS+=-I. -I${.CURDIR} -DHAVE_CONFIG_H
> +CFLAGS+=-I. -I${.CURDIR} -DHAVE_CONFIG_H -DHAVE_ASSERT_H
> SRCS= buf.c ccl.c dfa.c ecs.c filter.c gen.c main.c misc.c \
> nfa.c options.c parse.y regex.c scan.l scanflags.c \
> scanopt.c skel.c sym.c tables.c tables_shared.c \
Nope, you want to fix config.h
Re: signify: -z implies -q
On Fri, Jul 26, 2019 at 05:08:06AM +, Mike Burns wrote: > On 2019-07-26 03.02.43 +, Mike Burns wrote: > > Poking around signify and learned that `verifyzdata` calls `verifymsg` > > with `1` hardcoded in the `quiet` parameter. > > > > I do appreciate that there is a distinction between case 'z' setting > > `quiet = 1` vs `verifyzdata` passing `1` as an argument, so maybe this > > diff doesn't quiet capture a truth: > > Now that I've played with signify more, it's clear that mentioning -q is > quite far from from a complete statement of what -z is all about, Forget > about this diff, apologies. Yep. The verify part specifically only passes the gzip through if it's properly signed, and the signature is block-by-block... so it wouldn't be able to say anything positive until you reach the end of the file anyway.
Re: Host Header Redirection on openbsd.org
Well, the main issue I've seen so far is you flooding my mailboxen with lots of copies of the same useless mp4 video. What a douche.
Re: _pbuild user to have priority=5
On Sat, Nov 02, 2019 at 02:35:28PM +0100, Solene Rapenne wrote: > On Sat, Nov 02, 2019 at 01:18:53PM +, Stuart Henderson wrote: > > On 2019/11/01 19:16, Theo de Raadt wrote: > > > Ted Unangst wrote: > > > > > > > Theo de Raadt wrote: > > > > > What about all the other users who aren't in staff? > > > > > > > > > > I think the approach is right. Push non-interactive down. > > > > > > > > The same then for src build user? > > > > > > Well, that's different. Most of us building the src tree are waiting > > > eagerly for it to finish aren't we? > > > > That's the same for ports building! > > > > if you don't do anything else than compiling ports, that shouldn't be > slower. > If you are doing something else (GUI user, web server, community server > with people connected doing IRC) , then you don't get angry due to > unresponsive system. > > Lowering staff priority would only help the one user case. I agree with solene on that one. This is actually useful even if you're just building ports, because you get a more responsive text-editor and stuff like that which is useful when you're fixing things that broke while dpb is still going. I see a noticeable difference in vim showing me syntax coloring correctly while dpb is running. Source is somewhat different. make build/release is sequential by nature, as you can't really fix a part while something else is still building.
Re: [PATCH] make: implement jobserver and use it to avoid exponential behavior
On Tue, Nov 05, 2019 at 03:22:08PM +, Lauri Tirkkonen wrote:
> Hi,
>
> make has the concept of 'expensive' jobs: if the command line includes
> the word "make" in it, the command is considered expensive and other
> jobs are held until that job finishes. It does this to avoid exponential
> behavior when parallelizing with -j.
>
> Some other makes (eg. bmake and gmake) use a jobserver, ie. a pair of
> pipes between child makes and the top-level make: the top-level make
> provides job tokens to children via these pipes, which are returned when
> the jobs complete. This ensures that recursive makes only run a certain
> number of jobs in total at any one time, while achieving greater
> resource utilisation (no need to wait for an 'expensive' job to finish
> before starting others).
>
> This diff implements a similar jobserver in make. Like bmake and gmake,
> job tokens are provided by the top-level make (called "jobserver
> master"), used by children ("jobserver slaves") when they run commands,
> and returned when those commands finish. Like bmake, the internal,
> undocumented command line option to tell (possibly indirect, via
> MAKEFLAGS)) children about the file descriptor to use is -J, but unlike
> bmake and gmake, we use socketpair(AF_UNIX, SOCK_STREAM) instead of
> pipes, and therefore only need to pass one file descriptor to children
> instead of two.
Sorry, I missed your initial message (thanks to naddy@ for telling me about it)
A jobserver was considered in make before implementing expensive.
The main reason it was not done that way is that you can't guarantee a
file descriptor will make it through the intermediate shell.
In fact, POSIX doesn't guarantee anything, and a lot of shells actively close
file descriptors they don't know about.
Your example is not complicated enough, as your commands mean NO
shell will be spawned (everything will pass directly through to
submakes, which is BTW a large reason for implementing -C, which cuts
down on the number of intermediate processes).
I'm not sure I like added complexity in that part of make.
> I haven't yet built the entire tree with this, just cautiously probing
> the waters first to see if there is interest :)
I'd like some actual numbers on real world applications: what does it change
for the actual build through the full source tree. Areas like perl are where
the job explosion occurs and where the expensive heuristics got refined.
> Tested with the following Makefile structure:
>
> $ cat recursive/Makefile
> .PHONY: a b c d
>
> all: a b c d
>
> a:
> make -C a
> b:
> make -C a
> c:
> make -C a
> d:
> make -C a
> $ cat recursive/a/Makefile
> all: 1 2 3 4
>
> 1:
> sleep 1
> 2:
> sleep 2
> 3:
> sleep 3
> 4:
> sleep 4
>
> the speedup is as follows with -j4:
>
> before:
> make -C recursive -j4 0.01s user 0.10s system 0% cpu 16.097 total
> after:
> make -C recursive -j4 0.02s user 0.15s system 1% cpu 11.082 total
Comments on the patch proper:
> ---
> usr.bin/make/job.c | 317 +++-
> usr.bin/make/job.h | 7 +-
> usr.bin/make/main.c | 31 -
> usr.bin/make/main.h | 3 +
> 4 files changed, 290 insertions(+), 68 deletions(-)
>
> diff --git a/usr.bin/make/job.c b/usr.bin/make/job.c
> index ebbae5306da..b36aaeacac8 100644
> --- a/usr.bin/make/job.c
> +++ b/usr.bin/make/job.c
> @@ -90,11 +90,15 @@
> * Job_WaitWait for all running jobs to finish.
> */
>
> +#include
> #include
> #include
> +#include
> #include
> #include
> +#include
> #include
> +#include
> #include
> #include
> #include
> @@ -115,6 +119,7 @@
> #include "memory.h"
> #include "make.h"
> #include "buf.h"
> +#include "main.h"
>
> static int aborting = 0; /* why is the make aborting? */
> #define ABORT_ERROR 1 /* Because of an error */
> @@ -123,12 +128,15 @@ static int aborting = 0; /* why is the make
> aborting? */
>
> static int maxJobs;/* The most children we can run at once */
> static int nJobs; /* Number of jobs already allocated */
> -static bool no_new_jobs;/* Mark recursive shit so we shouldn't start
> - * something else at the same time
> - */
I don't think ditching expensive entirely is a good idea anyhow.
What happens if the jobserver fails ? I see it goes to err all the time.
BTW, going to err is bad. Make handles processes. Most errors MUST do
something sane, like waiting for processes to end in many many cases.
> +
> +struct jobserver {
> + int master; /* master deposits job tokens into this fd */
> + int slave; /* slaves consume job tokens from this fd */
> + volatile sig_atomic_t tokens; /*
Re: [PATCH] make: implement jobserver and use it to avoid exponential behavior
On Thu, Nov 14, 2019 at 05:30:34PM +0200, Lauri Tirkkonen wrote:
> > > @@ -347,10 +360,9 @@ print_errors(void)
> > > static void
> > > setup_signal(int sig)
> > > {
> > > - if (signal(sig, SIG_IGN) != SIG_IGN) {
> > > - (void)signal(sig, notice_signal);
> > > - sigaddset(, sig);
> > > - }
> > > + sigaction(sig, &(struct sigaction) { .sa_handler = notice_signal },
> > > + NULL);
> > > + sigaddset(, sig);
> > > }
> > Why did you change this ? this is gratuitous.
>
> Actually, it's not gratuitous, but it's subtle. signal() enables
> SA_RESTART, but we depend on EINTR returns in a couple places, so I did
> this. I should've mentioned this originally, sorry.
Err, thinking some more. You can't do that. This affects ALL of make's code
and not just this file. Some parts are belt-and-suspenders type and include
checks for EINTR, but NOWHERE near every part.
In particular, you *lose* stdio.h entirely.
Re: [PATCH] make: implement jobserver and use it to avoid exponential behavior
On Thu, Nov 14, 2019 at 05:30:34PM +0200, Lauri Tirkkonen wrote:
> Yeah, you're right, it reads like crap, I'll rewrite it. The gist of the
> thing is that 'expensive' heuristic is now only used to decide whether
> or not to print the failed command.
I object to that. Keep the "expensive" heuristics around.
The current -jn should still work *without* the jobserver mechanism.
> > > static void
> > > -loop_handle_running_jobs()
> > > +loop_handle_running_jobs(void)
> > > {
> > > while (runningJobs != NULL)
> > > handle_running_jobs();
> > > }
> > Yet another gratuitous change. There is already a perfectly fine prototype
> > that asserts (void).
>
> A function prototype with empty parentheses does not assert (void) in C
> (but does in C++). However, you're correct about it not being related to
> this change.
See above. There are prototypes for ALL static functions in that file.
You don't need the void in the function definition when the function
declaration (a few hundred lines above) already has it.
> This was also a bit concerning to me, but that's what bmake/gmake also
> are doing (albeit with more fd's). Eventually the potential gains
> outweighed my concerns: every SUBDIR in bsd.subdir.mk is currently built
> serially, which has quite a large impact on how many jobs are running at
> any one time.
>
> An alternative could be to use unix sockets with paths instead and pass
> the path through the environment, but I'm not convinced I like that any
> better.
There is the huge problem of where you put your paths. NFS comes in the
way AND socket names are limited to 256 characters.
Re: [PATCH] add support for versions with '-' before a/b/rc
On Fri, Dec 13, 2019 at 02:19:26PM +0100, Matija Skala wrote:
> Dne petek, 13. december 2019 ob 13:34:57 CET je Marc Espie napisal(a):
> > On Fri, Dec 13, 2019 at 08:45:08AM +0100, Jasper Lievisse Adriaanse wrote:
> > > Hello Matija,
> > >
> > > Could you please provide a testcase for inclusion in
> > > src/regress/usr.bin/pkg-config too? Also, is there a particular port or
> > > pkg-config file in the wild that you ran into which exhibits this
> > > particular pattern?
> > I'm going to ask more strongly: is this something that mainstream pkg-config
> > does ?
> pkgconf does do this. Not sure about pkg-config
> >
> > Also, pedantically, the change is wrong. You should always quote non-alpha
> > characters when they stand for themselves. Yes, that includes \-
> >
> Right, here a new patch
> diff --git a/regress/usr.bin/pkg-config/pcdir/alpha2.pc b/regress/usr.bin/pkg-
> config/pcdir/alpha2.pc
> new file mode 100644
> index 0..7958d3717
> --- /dev/null
> +++ b/regress/usr.bin/pkg-config/pcdir/alpha2.pc
> @@ -0,0 +1,4 @@
> +Name: alpha suffix test
> +Description: pkg-config(1) regress file
> +Version: 0.1.0-alpha2
> +Libs: -lalpha2
> diff --git a/regress/usr.bin/pkg-config/pcdir/beta2.pc b/regress/usr.bin/pkg-
> config/pcdir/beta2.pc
> new file mode 100644
> index 0..cc1959848
> --- /dev/null
> +++ b/regress/usr.bin/pkg-config/pcdir/beta2.pc
> @@ -0,0 +1,3 @@
> +Name: beta suffix test
> +Description: pkg-config(1) regress file
> +Version: 0.1.0-beta2
> diff --git a/regress/usr.bin/pkg-config/pcdir/rc2.pc b/regress/usr.bin/pkg-
> config/pcdir/rc2.pc
> new file mode 100644
> index 0..499242745
> --- /dev/null
> +++ b/regress/usr.bin/pkg-config/pcdir/rc2.pc
> @@ -0,0 +1,3 @@
> +Name: rc suffix test
> +Description: pkg-config(1) regress file
> +Version: 0.1.0-rc2
> diff --git a/usr.bin/pkg-config/pkg-config b/usr.bin/pkg-config/pkg-config
> index c1aab9576..1102696af 100644
> --- a/usr.bin/pkg-config/pkg-config
> +++ b/usr.bin/pkg-config/pkg-config
> @@ -674,13 +674,13 @@ sub compare
> # is there a valid non-numeric suffix to deal with later?
> # accepted are (in order): a(lpha) < b(eta) < rc < ' '.
> # suffix[0] is the 'alpha' part, suffix[1] is the '1' part in 'alpha1'.
> - if ($a =~ s/(rc|beta|b|alpha|a)(\d+)$//) {
> + if ($a =~ s/\-?(rc|beta|b|alpha|a)(\d+)$//) {
> say_debug("valid suffix $1$2 found in $a$1$2.");
> $suffix_a[0] = $1;
> $suffix_a[1] = $2;
> }
>
> - if ($b =~ s/(rc|beta|b|alpha|a)(\d+)$//) {
> + if ($b =~ s/\-?(rc|beta|b|alpha|a)(\d+)$//) {
> say_debug("valid suffix $1$2 found in $b$1$2.");
> $suffix_b[0] = $1;
> $suffix_b[1] = $2;
>
>
>
>
Fair enough. I'm okay with it then.
Re: [PATCH] add support for versions with '-' before a/b/rc
On Fri, Dec 13, 2019 at 08:45:08AM +0100, Jasper Lievisse Adriaanse wrote:
> Hello Matija,
>
> Could you please provide a testcase for inclusion in
> src/regress/usr.bin/pkg-config too?
> Also, is there a particular port or pkg-config file in the wild that you ran
> into which exhibits this particular pattern?
I'm going to ask more strongly: is this something that mainstream pkg-config
does ?
Also, pedantically, the change is wrong. You should always quote non-alpha
characters when they stand for themselves. Yes, that includes \-
> > On 12 Dec 2019, at 18:28, Matija Skala wrote:
> >
> > From fa66eb42d0bd2fec7b364644684e6a4cc9ae9baa Mon Sep 17 00:00:00 2001
> > From: Matija Skala
> > Date: Thu, 28 Nov 2019 19:24:42 +0100
> > Subject: [PATCH] add support for versions with '-' before a/b/rc
> >
> > ---
> > usr.bin/pkg-config/pkg-config | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/usr.bin/pkg-config/pkg-config b/usr.bin/pkg-config/pkg-config
> > index 6dfbd3224eb..c050e9b058e 100644
> > --- a/usr.bin/pkg-config/pkg-config
> > +++ b/usr.bin/pkg-config/pkg-config
> > @@ -674,13 +674,13 @@ sub compare
> > # is there a valid non-numeric suffix to deal with later?
> > # accepted are (in order): a(lpha) < b(eta) < rc < ' '.
> > # suffix[0] is the 'alpha' part, suffix[1] is the '1' part in 'alpha1'.
> > - if ($a =~ s/(rc|beta|b|alpha|a)(\d+)$//) {
> > + if ($a =~ s/-?(rc|beta|b|alpha|a)(\d+)$//) {
> > say_debug("valid suffix $1$2 found in $a$1$2.");
> > $suffix_a[0] = $1;
> > $suffix_a[1] = $2;
> > }
> >
> > - if ($b =~ s/(rc|beta|b|alpha|a)(\d+)$//) {
> > + if ($b =~ s/-?(rc|beta|b|alpha|a)(\d+)$//) {
> > say_debug("valid suffix $1$2 found in $b$1$2.");
> > $suffix_b[0] = $1;
> > $suffix_b[1] = $2;
> >
> >
>
>
Re: patch: make sdiff tty-aware
On Wed, Dec 11, 2019 at 01:15:40PM -0700, Theo de Raadt wrote: > I'm not sure I understand the goal of the signal handler. > > sdiff is moving forward through the file, only. If you are in a pager, > you want to increase the width for the later output not yet visible? > the normal way one does that in programs which don't backtrack and > re-output, is by restarting the program. > > You don't want a mix of shorter, then longer. If you are using a pager, > I suspect at least one of the next lines is already be buffered out. So > I don't understand how this will create output which looks correct. > > I think you should remove the signal handler. Well, in the sysmerge use-case, you see the first line showing the $OpenBSD$ prompt usually. That's often when you realize your terminal is really small, and you will miss the next lines. The $OpenBSD$ line is reasonably unambiguous, as the rev number is fairly prominent. The next ones will need the space. Admittedly, it could be smarter, and react during actual user interaction, or offer to redraw the last block with a specific command ?
Re: patch: make sdiff tty-aware
On Thu, Dec 12, 2019 at 02:51:18AM -0700, Theo de Raadt wrote: > Marc Espie wrote: > > > On Wed, Dec 11, 2019 at 01:15:40PM -0700, Theo de Raadt wrote: > > > I'm not sure I understand the goal of the signal handler. > > > > > > sdiff is moving forward through the file, only. If you are in a pager, > > > you want to increase the width for the later output not yet visible? > > > the normal way one does that in programs which don't backtrack and > > > re-output, is by restarting the program. > > > > > > You don't want a mix of shorter, then longer. If you are using a pager, > > > I suspect at least one of the next lines is already be buffered out. So > > > I don't understand how this will create output which looks correct. > > > > > > I think you should remove the signal handler. > > > > Well, in the sysmerge use-case, you see the first line showing the > > $OpenBSD$ prompt usually. That's often when you realize your terminal is > > really small, and you will miss the next lines. The $OpenBSD$ line is > > reasonably unambiguous, as the rev number is fairly prominent. > > > > The next ones will need the space. > > > > Admittedly, it could be smarter, and react during actual user interaction, > > or offer to redraw the last block with a specific command ? > > That you have to dig for such a narrow example really demonstrates > the pointlessness of the signal handler. I didn't have to dig... it's the specific usecase I have for sdiff3!
patch: make sdiff tty-aware
This patch allows sdiff to auto-detect tty width,
by passing -w auto
More importantly, sdiff will adjust its variables on the fly for
subsequent lines (I don't know if redrawing the current
lines when the tty changes is advisable).
It's pretty straightforward, tested thru sysmerge.
Doesn't seem it's standardized anywhere, I'm not sure having a functionality
that's
not in gnu-sdiff would be an issue ?
Index: sdiff.c
===
RCS file: /cvs/src/usr.bin/sdiff/sdiff.c,v
retrieving revision 1.37
diff -u -p -r1.37 sdiff.c
--- sdiff.c 28 Sep 2018 18:21:52 - 1.37
+++ sdiff.c 11 Dec 2019 16:42:25 -
@@ -17,12 +17,14 @@
#include
#include
#include
+#include
#include
#include
#include
#include
#include
#include
+#include
#include "common.h"
#include "extern.h"
@@ -43,7 +45,9 @@ struct diffline {
};
static void astrcat(char **, const char *);
+static void compute_width(void);
static void enqueue(char *, char, char *);
+static void find_width(void);
static char *mktmpcpy(const char *);
static void freediff(struct diffline *);
static void int_usage(void);
@@ -55,12 +59,16 @@ static void printd(FILE *, size_t);
static void println(const char *, const char, const char *);
static void processq(void);
static void prompt(const char *, const char *);
+static void recompute_width(void);
__dead static void usage(void);
+static void window_changed(int);
static char *xfgets(FILE *);
SIMPLEQ_HEAD(, diffline) diffhead = SIMPLEQ_HEAD_INITIALIZER(diffhead);
size_t line_width;/* width of a line (two columns and divider) */
size_t width; /* width of each column */
+size_t wflag = WIDTH;
+volatile sig_atomic_t got_signal = 0;
size_t file1ln, file2ln; /* line number of file1 and file2 */
int Iflag = 0; /* ignore sets matching regexp */
int lflag; /* print only left column for identical lines */
@@ -153,11 +161,47 @@ FAIL:
exit(2);
}
+static void
+window_changed(int sig __attribute__((__unused__)))
+{
+ got_signal = 1;
+}
+
+static void
+find_width(void)
+{
+ struct winsize ws;
+
+ if (ioctl(0, TIOCGWINSZ, ) != 0) {
+ wflag = WIDTH;
+ signal(SIGWINCH, SIG_DFL);
+ } else
+ wflag = ws.ws_col;
+}
+
+static void
+recompute_width(void)
+{
+ find_width();
+ compute_width();
+}
+
+static void
+compute_width(void)
+{
+ /* Subtract column divider and divide by two. */
+ width = (wflag - 3) / 2;
+ /* Make sure line_width can fit in size_t. */
+ if (width > (SIZE_MAX - 3) / 2)
+ errx(2, "width is too large: %zu", width);
+ line_width = width * 2 + 3;
+}
+
int
main(int argc, char **argv)
{
FILE *diffpipe, *file1, *file2;
- size_t diffargc = 0, wflag = WIDTH;
+ size_t diffargc = 0;
int ch, fd[2], status;
pid_t pid;
const char *outfile = NULL;
@@ -236,6 +280,11 @@ main(int argc, char **argv)
diffargv[diffargc++] = "-w";
break;
case 'w':
+ if (strcmp(optarg, "auto") == 0) {
+ signal(SIGWINCH, window_changed);
+ find_width();
+ break;
+ }
wflag = strtonum(optarg, WIDTH_MIN,
INT_MAX, );
if (errstr)
@@ -273,7 +322,7 @@ main(int argc, char **argv)
if (unveil(_PATH_BSHELL, "x") == -1)
err(2, "unveil");
}
- if (pledge("stdio rpath wpath cpath proc exec", NULL) == -1)
+ if (pledge("stdio rpath wpath cpath proc exec tty", NULL) == -1)
err(2, "pledge");
/*
@@ -302,12 +351,7 @@ main(int argc, char **argv)
/* Add NULL to end of array to indicate end of array. */
diffargv[diffargc++] = NULL;
- /* Subtract column divider and divide by two. */
- width = (wflag - 3) / 2;
- /* Make sure line_width can fit in size_t. */
- if (width > (SIZE_MAX - 3) / 2)
- errx(2, "width is too large: %zu", width);
- line_width = width * 2 + 3;
+ compute_width();
if (pipe(fd))
err(2, "pipe");
@@ -522,6 +566,11 @@ static void
println(const char *s1, const char div, const char *s2)
{
size_t col;
+
+ if (got_signal) {
+ got_signal = 0;
+ recompute_width();
+ }
/* Print first column. Skips if s1 == NULL. */
col = 0;
Re: [PATCH] make: implement jobserver and use it to avoid exponential behavior
On Wed, Nov 27, 2019 at 05:31:48PM +0200, Lauri Tirkkonen wrote: > I'll give you some more background that I maybe should have given > earlier already: in my hobby OS Unleashed, we use bmake and earlier did > some (slightly hacky) modifications to subdir.mk to enable paralellizing > jobs in multiple subdirs at the same time. I don't remember the exact > numbers since it's been quite some time, but I was able to cut the base > build time by *a lot* with just that change -- during the build there > were previously a lot of idle CPUs. Those results served as motivation > for me to start looking into doing similar things in OpenBSD, and having > a jobserver is sort of the first step there. I did experiment with something similar a while back: reorganizing a large part of usr.bin or usr.sbin to just be one single variation of bsd.prog.mk with multiple progs and multiple object files... works just fine for, say 95% of the binaries in those directories (considering there are lots of directories with one single C file or two C files, this sounds like a gain for make -jN, right ?) End result: no measurable gain. That on 4 cpu boxes at the time. There are SMP issues to solve before this actually yields any useful result...
Re: [PATCH] make: implement jobserver and use it to avoid exponential behavior
On Wed, Nov 27, 2019 at 06:21:01PM +0200, Lauri Tirkkonen wrote: > All that said I do understand if there is reluctance to merge the > jobserver stuff since it doesn't actually help the current situation in > most cases. Nevertheless it has been personally beneficial to me in > identifying areas of improvement, even if those are nothing new to > OpenBSD developers. Oh, I'm not opposed to merging a jobserver once it's roughly as stable as what we have. My concerns are: - it should work with all of src and xenocara (once that's done, I can do ports) - it should not interfere with other jobservers in other makes. - it does not replace the expensive jobs heuristics. - it does not render make harder to maintain and evolve. Especially since, as I've said before, there's no actual guarantee anywhere in POSIX that shells will cooperate with it. I don't have enough time to do everything I want, but it's well known that there are still issues with make -j compared to sequential make. If you look at cvs history, I've completely rewritten job.c a few times. And I had a presentation at eurobsdcon about what's still missing a few years ago. I've been swamped with changes in dpb/bsd.port.mk/pkg_add... recently, but it's still something I want to get back to eventually ! One reason that work goes slowly is because make was a seriously entwined series of things that all depend on each other... It still is, but slightly less so, and thus anything that's not squeaky clean is unlikely to get in. (hence me wanting the jobserver stuff to be reasonably self-contained and not interfering with the rest of make)
Re: [PATCH] make: implement jobserver and use it to avoid exponential behavior
On Fri, Nov 15, 2019 at 03:29:29PM +0200, Lauri Tirkkonen wrote: > On Fri, Nov 15 2019 15:24:57 +0200, Lauri Tirkkonen wrote: > > Your points are valid and I agree with them completely. There are > > clearly problems with lock contention, > > and I should mention here that I would probably not have observed the > magnitude of these problems had I not made make start more jobs at once > in the first place. So while the make/mk diffs need more work, we've > already identified more areas of improvement as a result of trying them > out. But one thing at a time. :) Seriously, as much as what you're doing is interesting, you've not identified anything new in THAT specific area. We've run enough build configurations to know about those scalability issues for years. It's cool that you found new races that are hidden by expensive jobs, though, that much I fully agree with.
change to bsd.port.mk to help debug packages a bit
This should get rid of the weird error (don't know how to make
.../all/debug-*.tgz) when switching from !DEBUG_PACKAGES to DEBUG_PACKAGES.
Basically, this introduces a "build two targets at once" in bsd.port.mk.
make has some glue to figure out whether this is a "duplicate the work" or
"one target builds two things" (already used for yacc and friends), by
looking for variations on $@... which includes $@D for better or worse, hence
the small tweak.
This should work on all settings, including running make -jN package
(which doesn't make a lot of sense, but whatever).
Please check/test
Index: bsd.port.mk
===
RCS file: /cvs/ports/infrastructure/mk/bsd.port.mk,v
retrieving revision 1.1505
diff -u -p -r1.1505 bsd.port.mk
--- bsd.port.mk 26 Nov 2019 11:49:02 - 1.1505
+++ bsd.port.mk 27 Nov 2019 16:49:05 -
@@ -1183,6 +1183,7 @@ _EXCLUDE_DEBUG_PLISTS = ${_WRKDEBUG} ${_
PKG_ARGS${_S} += -A'${PKG_ARCH${_S}}'
_pkg${_S} = ${_PKGFILE${_S}}
+_pkg_cookie${_S} = ${_PACKAGE_COOKIE${_S}}
. if ${DEBUG_PACKAGES:M${_S}}
_DBG_PKG_ARGS${_S} := ${PKG_ARGS${_S}}
@@ -1194,6 +1195,7 @@ _DBG_PKG_ARGS${_S} += -DFULLPKGPATH=debu
_DBG_PKG_ARGS${_S} += -f ${_WRKDEBUG}/${PLIST${_S}:T}
_EXCLUDE_DEBUG_PLISTS += ${_WRKDEBUG}/${PLIST${_S}:T}
_pkg${_S} += ${_DBG_PKGFILE${_S}}
+_pkg_cookie${_S} += ${_DBG_PACKAGE_COOKIE${_S}}
. endif
# Finish filling out package command, and package dependencies
@@ -2073,8 +2075,8 @@ ${_CACHE_REPO}${_p}:
# The real package
-${_PACKAGE_COOKIE${_S}}:
- @${_PBUILD} install -d ${PACKAGE_REPOSITORY_MODE} ${@D} ${_TMP_REPO}
+${_pkg_cookie${_S}}:
+ @${_PBUILD} install -d ${PACKAGE_REPOSITORY_MODE} ${_PKG_REPO${_S}}
${_TMP_REPO}
. if ${FETCH_PACKAGES:L} != "no" &&
!defined(_TRIED_FETCHING_${_PACKAGE_COOKIE${_S}})
@${_INSTALL_CACHE_REPO} ${_CACHE_REPO}
@cd ${.CURDIR}; gotit=true; for p in ${_pkg${_S}}; do \
Re: grep -R with no path
On Mon, Dec 02, 2019 at 08:31:02AM +, Miod Vallat wrote:
> grep(1), when invoked with the -R option but no path, displays a
> "recursive search of stdin" warning and acts as if -R had not been
> specified.
>
> GNU grep, in that case, will perform a recursive search in the current
> directory, i.e. uses an implicit "." path if none is given.
>
> This is IMO a much better behaviour. What about the following diff?
>
> Index: grep.c
> ===
> RCS file: /OpenBSD/src/usr.bin/grep/grep.c,v
> retrieving revision 1.62
> diff -u -p -r1.62 grep.c
> --- grep.c7 Oct 2019 20:04:00 - 1.62
> +++ grep.c2 Dec 2019 08:27:09 -
> @@ -473,8 +473,12 @@ main(int argc, char *argv[])
> ++argv;
> }
>
> - if (Rflag && argc == 0)
> - warnx("warning: recursive search of stdin");
> + if (Rflag && argc == 0) {
> + /* default to . if no path given */
> + static char *dot_argv[] = { ".", NULL };
> + argv = dot_argv;
> + argc = 1;
> + }
> if (Eflag)
> cflags |= REG_EXTENDED;
> if (Fflag)
>
>
I concur, I like it better as well.
-R isn't in POSIX anyway, so we don't have to worry about standard.
Re: debug packages: let strip do the stripping
On Mon, Nov 25, 2019 at 04:11:53PM +0200, Paul Irofti wrote:
> Hi,
>
> Few people complained (hi landry@!) that stripped binaries are slightly
> larger now than they used to be when debug packages are enabled.
>
> My investigations show that this is because objcopy --strip-debug is
> less efficient than plain strip(1) which is what we use for non-debug
> packages.
>
> Reintroducing strip(1) does not affect current debug packages behaviour
> in my experience. The link to the debug symbols is still there and
> egdb(1) still loads it automatically and displays all the debug info.
>
> OK?
>
> Paul
>
> Index: bin/build-debug-info
> ===
> RCS file: /cvs/ports/infrastructure/bin/build-debug-info,v
> retrieving revision 1.22
> diff -u -p -u -p -r1.22 build-debug-info
> --- bin/build-debug-info 19 Nov 2019 15:49:30 - 1.22
> +++ bin/build-debug-info 25 Nov 2019 14:06:34 -
> @@ -263,7 +263,7 @@ print {$self->{mk}} << 'EOPREAMBLE';
> OBJCOPY_RULE = ${INSTALL_DATA_DIR} ${@D} && \
> echo "> Copy debug info from $? to $@" && \
> objcopy --only-keep-debug $? $@ && \
> -objcopy --strip-debug $? && \
> +strip $? && \
> objcopy --add-gnu-debuglink=$@ $? && \
> touch $@
>
>
Okay
Re: [PATCH] make: implement jobserver and use it to avoid exponential behavior
On Wed, Nov 27, 2019 at 12:42:48PM +0200, Lauri Tirkkonen wrote: > In a lesson to always proof-read *before* sending your message: > > On Wed, Nov 27 2019 12:31:51 +0200, Lauri Tirkkonen wrote: > > - a full release build with the jobserver enabled passed after > >disabling gnu/usr.bin/binutils > > this should, in fact, read "gnu/usr.bin/binutils-2.17". binutils builds > fine. We would need to investigate this. Instead of disabling them, send logs
Re: mention /etc/examples/ in bgpf.conf(5)/bgpd(8)
On Sun, Feb 09, 2020 at 01:37:33PM +, Jason McIntyre wrote: > On Sun, Feb 09, 2020 at 02:27:23PM +0100, Marc Espie wrote: > > On Sun, Feb 09, 2020 at 12:41:43AM +0100, Sebastian Benoit wrote: > > > Ingo Schwarze(schwa...@usta.de) on 2020.02.09 00:33:06 +0100: > > > > Hi, > > > > > > > > Jason McIntyre wrote on Sat, Feb 08, 2020 at 10:15:08PM +: > > > > > > > > > - i'm ok with adding the path to these files to a FILES section > > > > > > > > So, here is a specific patch for bgpf.conf(5) and bgpd(8) such > > > > that we can agree on a general direction for one case where > > > > the example file is particularly important. > > > > > > > > Once the direction is agreed, applying it to the relatively > > > > small number of other cases is likely not difficult. > > > > > > > > OK? > > > > Ingo > > > > > > I'm not against this as it only adds to the manpage, but an > > > alternative could be to just tell users (in afterboot(8)?) that there is > > > /etc/examples for them to look at and see whats there? > > > > > > Otherwise ok. > > > > I still think it's a good idea to put it in afterboot(8). > > That was the patch I sent to a few people that obviously triggered Ingo. > > > > As for /etc/examples, I still think that directory has merits. > > It's WAY easier to copy an example file and uncomment/tweak the parts you > > need while skimming through the manpage than creating one from scratch. > > > > Also, cut from an EXAMPLE in the manpage is distasteful. > > > > Yeah, it's more info that has to be kept in synch. So what ? It's still > > useful, so it should stay. > > > > Here's an updated version, taking Theo's remarks into account. > > > > hi. i'm fine with your text, but it would be better placed in the > section "Daemons" (yes i'm aware not all the files are for daemons) > or a new section in "FURTHER CHANGES". > > jmc Sorry, I saw your message a bit late. I don't think it's a good idea. Specifically, that text should come in as early as possible. "FURTHER CHANGES" sound like a section near the end. "Daemons" also comes very late in the game (after ntpd, after sysctl.conf, which do have /etc/examples scripts) also, you should update, as your email setup is broken. any direct email to you have come back with : 553 ORCPT address syntax error which according to Stuart, has been fixed in recent snaps.
Re: MAKE: some older keywords
So I did a full bulk build with the following diff.
No failure due to old keyword.
This contains exactly:
- abort directly instead of setting pgn->must_make = false
- tag as "SPECIAL_DEPRECATED" all old keywords
- remove the conditionals with those OP. We just keep OP_INVISIBLE because
it's actually also used by cohorts (those pesky :: dependencies in imake)
diff --git a/compat.c b/compat.c
index fd78d78..af18ce3 100644
--- a/compat.c
+++ b/compat.c
@@ -101,24 +101,13 @@ CompatMake(void *gnp, /* The node to make */
if (pgn == NULL)
pgn = gn;
- if (pgn->type & OP_MADE) {
- sib = gn;
- do {
- sib->mtime = gn->mtime;
- sib->built_status = UPTODATE;
- sib = sib->sibling;
- } while (sib != gn);
- }
-
switch(gn->built_status) {
case UNKNOWN:
/* First mark ourselves to be built, then apply whatever
* transformations the suffix module thinks are necessary.
* Once that's done, we can descend and make all our children.
* If any of them has an error but the -k flag was given,
-* our 'must_make' field will be set false again. This is our
-* signal to not attempt to do anything but abort our
-* parent as well. */
+* we will abort. */
gn->must_make = true;
gn->built_status = BUILDING;
/* note that, in case we have siblings, we only check all
@@ -132,10 +121,9 @@ CompatMake(void *gnp, /* The node to make */
sib = sib->sibling;
} while (sib != gn);
- if (!gn->must_make) {
+ if (gn->built_status == ABORTED) {
Error("Build for %s aborted", gn->name);
- gn->built_status = ABORTED;
- pgn->must_make = false;
+ pgn->built_status = ABORTED;
return;
}
@@ -228,12 +216,10 @@ CompatMake(void *gnp, /* The node to make */
if (DEBUG(MAKE))
printf("update time: %s\n",
time_to_string(>mtime));
- if (!(gn->type & OP_EXEC)) {
- pgn->child_rebuilt = true;
- Make_TimeStamp(pgn, gn);
- }
+ pgn->child_rebuilt = true;
+ Make_TimeStamp(pgn, gn);
} else if (keepgoing)
- pgn->must_make = false;
+ pgn->built_status = ABORTED;
else {
print_errors();
exit(1);
@@ -242,22 +228,19 @@ CompatMake(void *gnp, /* The node to make */
case ERROR:
/* Already had an error when making this beastie. Tell the
* parent to abort. */
- pgn->must_make = false;
+ pgn->built_status = ABORTED;
break;
case BUILDING:
Error("Graph cycles through %s", gn->name);
gn->built_status = ERROR;
- pgn->must_make = false;
+ pgn->built_status = ABORTED;
break;
case REBUILT:
- if ((gn->type & OP_EXEC) == 0) {
- pgn->child_rebuilt = true;
- Make_TimeStamp(pgn, gn);
- }
+ pgn->child_rebuilt = true;
+ Make_TimeStamp(pgn, gn);
break;
case UPTODATE:
- if ((gn->type & OP_EXEC) == 0)
- Make_TimeStamp(pgn, gn);
+ Make_TimeStamp(pgn, gn);
break;
default:
break;
diff --git a/dump.c b/dump.c
index b3820eb..f35cacf 100644
--- a/dump.c
+++ b/dump.c
@@ -117,7 +117,7 @@ TargPrintNode(GNode *gn, bool full)
}
if (full) {
printf("# %d unmade prerequisites\n", gn->children_left);
- if (! (gn->type & (OP_JOIN|OP_USE|OP_EXEC))) {
+ if (! (gn->type & OP_USE)) {
if (!is_out_of_date(gn->mtime)) {
printf("# last modified %s: %s\n",
time_to_string(>mtime),
diff --git a/engine.c b/engine.c
index 535d8dd..4d7c128 100644
--- a/engine.c
+++ b/engine.c
@@ -241,7 +241,7 @@ void
Job_Touch(GNode *gn)
{
handle_all_signals();
- if (gn->type & (OP_JOIN|OP_USE|OP_EXEC|OP_OPTIONAL|OP_PHONY)) {
+ if (gn->type & (OP_USE|OP_OPTIONAL|OP_PHONY)) {
/*
* .JOIN, .USE, and .OPTIONAL targets are "virtual" targets
* and, as such, shouldn't really be created.
@@ -347,7 +347,7 @@ Make_DoAllVar(GNode *gn)
waitpid vs ptrace
So, basically, if we start arbitrary commands, then
the classic loop
/* Wait for the child to exit. */
while (waitpid(cpid, , 0) == -1 && errno == EINTR)
continue;
is not quite enough.
See the small note in manpages (not only us, but everyone):
WIFSTOPPED(status)
True if the process has not terminated, but has stopped and can
be restarted. This macro can be true only if the wait call
specified the WUNTRACED option or if the child process is being
traced (see ptrace(2)).
this means that somebody could run a command that forks, and then its child
(our grand-child) could use ptrace on its parent, and then we would get
a notification with WIFSTOPPED (assuming the current kernel bug is fixed).
e.g., something like that:
while (1) {
if (waitpid(cpid, , 0) == -1) {
if (errno == EINTR)
continue;
} else {
if (WIFSTOPPED(status))
continue;
}
break;
}
or should we just assume no-one will be nasty enough to do that ?
Re: sysupgrade should check for downgrades
On Thu, Jan 30, 2020 at 11:14:37AM +, Stuart Henderson wrote:
> Putting this here for discussion... good idea? bad idea? does it need
> more checks for expected file contents?
>
> Index: sysupgrade.sh
> ===
> RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
> retrieving revision 1.37
> diff -u -p -r1.37 sysupgrade.sh
> --- sysupgrade.sh 26 Jan 2020 22:08:36 - 1.37
> +++ sysupgrade.sh 30 Jan 2020 10:56:52 -
> @@ -131,6 +131,7 @@ cd ${SETSDIR}
>
> echo "Fetching from ${URL}"
> unpriv -f SHA256.sig ftp -N sysupgrade -Vmo SHA256.sig ${URL}SHA256.sig
> +unpriv -f BUILDINFO ftp -N sysupgrade -Vmo BUILDINFO ${URL}BUILDINFO
>
> _KEY=openbsd-${_KERNV[0]%.*}${_KERNV[0]#*.}-base.pub
> _NEXTKEY=openbsd-${NEXT_VERSION%.*}${NEXT_VERSION#*.}-base.pub
> @@ -147,11 +148,26 @@ esac
> unpriv -f SHA256 signify -Ve -p "${SIGNIFY_KEY}" -x SHA256.sig -m SHA256
> rm SHA256.sig
>
> +unpriv cksum -qC SHA256 BUILDINFO
> +
> if cmp -s /var/db/installed.SHA256 SHA256 && ! $FORCE; then
> echo "Already on latest snapshot."
> exit 0
> fi
>
> +if [[ -r /var/db/installed.BUILDINFO ]] && ! $FORCE; then
> + read _skip _skip _oldbuildtime _skip < /var/db/installed.BUILDINFO
> + read _skip _skip _newbuildtime _skip < BUILDINFO
> + if [[ $_newbuildtime -lt $_oldbuildtime ]]; then
> + echo "Snapshot on mirror is older than installed version!"
> + exit 1
> + fi
> + if [[ $_newbuildtime -eq $_oldbuildtime ]]; then
> + echo "Already on latest snapshot? Mismatch between BUILDINFO
> and SHA256?"
> + exit 1
> + fi
> +fi
> +
> # INSTALL.*, bsd*, *.tgz
> SETS=$(sed -n -e 's/^SHA256 (\(.*\)) .*/\1/' \
> -e '/^INSTALL\./p;/^bsd/p;/\.tgz$/p' SHA256)
> @@ -187,9 +203,14 @@ Set name(s) = done
> Directory does not contain SHA256.sig. Continue without verification = yes
> __EOT
>
> +# XXX should be done in bsd.rd so that this is present for a clean install
> too
> +cat <<__EOT > /etc/rc.firsttime
> +cp /home/_sysupgrade/BUILDINFO /var/db/installed.BUILDINFO
> +__EOT
> +
> if ! ${KEEP}; then
> CLEAN=$(echo SHA256 ${SETS} | sed -e 's/ /,/g')
> - cat <<__EOT > /etc/rc.firsttime
> + cat <<__EOT >> /etc/rc.firsttime
> rm -f /home/_sysupgrade/{${CLEAN}}
> __EOT
> fi
>
>
I think it's a good idea. A "bad" mirror could intentionally keep
snapshots around waiting for a critical bug to hit us, and then swap back
to an older snapshot that would be vulnerable. It would probably get
noticed reasonably quick, but not before it could make some damage.
Re: [patch] /bin/cp: reduce scope variable
On Sat, Feb 01, 2020 at 04:57:26PM +0100, Ingo Schwarze wrote: > Hi Jeremie, > > Jeremie Courreges-Anglas wrote on Sat, Feb 01, 2020 at 01:37:32PM +0100: > > On Fri, Jan 31 2020, Ingo Schwarze wrote: > >> ngc...@gmail.com wrote on Fri, Jan 31, 2020 at 10:14:52PM +0900: > > >>> Reduce scope of a few variables. > > >> No, this contradicts OpenBSD coding style. > >> We want local variables declared up front in functions > >> such that you can see at one glance which variables exist. > > > Huh, this is your personal preference and I strongly disagree with > > making it an "official" stance (again). Reducing the scope of variables > > *quite often* helps reasoning about them. > > > > style(9) said something like that, and has been changed three years > > ago (for good IMO). > > > > revision 1.68 > > date: 2016/10/18 18:13:56; author: millert; state: Exp; lines: +2 -4; > > commitid: aPPHgmmA4hseZUFx; > > Don't tell the programmer not to put variable declarations inside > > blocks. OK guenther@ kettenis@ > > Oops. Sorry for forgetting that the recommendation was dropped and > for making you dig up the commit and thanks for reminding me. Since it came out in the open, I also like declaring variables as late as possible. Specifically, because the old-style (declare variables early, then give them a value when possible) tends to be unsafe. You forget about initializing variables, or you initialize them to something that doesn't make sense. I also don't think it's more readable to have every variable at the start of the function. The less space my eyes have to travel the better. Especially with functions that run 50 lines long. Also, reducing the scope of variables tends to be one of the first steps in untangling old atrocious code: reduce scope, figure out you got two separate blocks in your function with totally separate variables, and then you write a new function. Compilers aren't perfect yet, but we still deal with code that was written 20-30 years ago, back when they were even worse, and a lot of dirty tricks that were used back then are no longer needed. Reusing variables, having the compiler match your scopes in asm exactly, you name it. These days, scope is only a semantic promise that this variable only matters in THAT specific block of code. The stronger the promise, the more readable the code.
Re: mention /etc/examples/ in bgpf.conf(5)/bgpd(8)
On Sun, Feb 09, 2020 at 12:41:43AM +0100, Sebastian Benoit wrote: > Ingo Schwarze(schwa...@usta.de) on 2020.02.09 00:33:06 +0100: > > Hi, > > > > Jason McIntyre wrote on Sat, Feb 08, 2020 at 10:15:08PM +: > > > > > - i'm ok with adding the path to these files to a FILES section > > > > So, here is a specific patch for bgpf.conf(5) and bgpd(8) such > > that we can agree on a general direction for one case where > > the example file is particularly important. > > > > Once the direction is agreed, applying it to the relatively > > small number of other cases is likely not difficult. > > > > OK? > > Ingo > > I'm not against this as it only adds to the manpage, but an > alternative could be to just tell users (in afterboot(8)?) that there is > /etc/examples for them to look at and see whats there? > > Otherwise ok. I still think it's a good idea to put it in afterboot(8). That was the patch I sent to a few people that obviously triggered Ingo. As for /etc/examples, I still think that directory has merits. It's WAY easier to copy an example file and uncomment/tweak the parts you need while skimming through the manpage than creating one from scratch. Also, cut from an EXAMPLE in the manpage is distasteful. Yeah, it's more info that has to be kept in synch. So what ? It's still useful, so it should stay. Here's an updated version, taking Theo's remarks into account. Index: afterboot.8 === RCS file: /cvs/src/share/man/man8/afterboot.8,v retrieving revision 1.164 diff -u -p -r1.164 afterboot.8 --- afterboot.8 28 Sep 2018 18:21:31 - 1.164 +++ afterboot.8 9 Feb 2020 13:26:29 - @@ -60,6 +60,10 @@ command, type: Administrators will rapidly become more familiar with .Ox if they get used to using the high quality manual pages. +.Pp +Some base programs and subsystems also come with sample configuration +files in +.Pa /etc/examples . .Ss Errata By the time that you have installed your system, it is possible that bugs in the release have been found.
MAKE: redux patch
So my development branch is getting a bit too far
ahead compared to what's committed.
Here's a big diff to test. Comments as to what's going on
and the changes this contains:
- buffer changes: add support for "permanent static buffers"
that don't get reinit'd all the time (that's a mostly trivial
change)
- parser change: SPECIAL_TARGET and SPECIAL_SOURCE are not
really needed (figured out in netbsd)
- (related): create the special nodes once in a simpler way
- a bit of reorg: take the code for expand_children out
of suff.c proper, as this file is always fairly unwieldy.
This only requires Link_Parent, and find_best_path, which
are suff-independent.
- engine change: stop mixing parallel and compat so much.
In particular, don't run Make_Update when in compat mode (which
would happen because handle_all_signals goes into the main loop)
- (related): abstract the engine running into operations in
'enginechoice.c' so that it's perfectly clear what part is compatMake
and what part is not. Have it report out_of_date and errors in
a simple way. Don't mix up .BEGIN/.END handling.
- (related): job.c determines whether to use the expensive heuristics
or not depending on how many jobs we run.
- jobrunner cleanup: we have no need for maxjobs, it's enough to move
stuff around from available into running or error... do the move to
error later so that everything is in the same location.
- with the above changes: the special case of running_one_job is no
longer needed at all.
- feature coming from bmake: treat .BEGIN/.END more as normal targets.
More specifically, just invoke the engine on them if they exist.
This makes it possible to have
.BEGIN: somethingelse
which does make a lot of sense, actually.
I would really need this to go in so that I can keep pushing forward.
I think I might take out the "old" parallel engine (which is somewhat
broken, and frankly, I don't get how it can work) because modifying
compat to actually queue stuff and build it is a distinct possibility
now. e.g., having an actual parallel engine that works.
diff --git a/Makefile b/Makefile
index 0cd84fc..90747de 100644
--- a/Makefile
+++ b/Makefile
@@ -16,8 +16,8 @@ CFLAGS+=${CDEFS}
HOSTCFLAGS+=${CDEFS}
SRCS= arch.c buf.c cmd_exec.c compat.c cond.c dir.c direxpand.c dump.c \
- engine.c \
- error.c for.c init.c job.c lowparse.c main.c make.c memory.c parse.c \
+ engine.c enginechoice.c error.c expandchildren.c \
+ for.c init.c job.c lowparse.c main.c make.c memory.c parse.c \
parsevar.c str.c stats.c suff.c targ.c targequiv.c timestamp.c \
var.c varmodifiers.c varname.c
diff --git a/arch.c b/arch.c
index 85e8e7e..02c18b5 100644
--- a/arch.c
+++ b/arch.c
@@ -195,11 +195,10 @@ bool
Arch_ParseArchive(const char **line, Lst nodes, SymTable *ctxt)
{
bool result;
- BUFFER expand;
+ static BUFFER expand;
- Buf_Init(, MAKE_BSIZE);
+ Buf_Reinit(, MAKE_BSIZE);
result = parse_archive(, line, nodes, ctxt);
- Buf_Destroy();
return result;
}
diff --git a/buf.c b/buf.c
index 74cbfe8..931bd7e 100644
--- a/buf.c
+++ b/buf.c
@@ -153,6 +153,15 @@ Buf_printf(Buffer bp, const char *fmt, ...)
bp->inPtr += n;
}
+void
+Buf_Reinit(Buffer bp, size_t size)
+{
+ if (bp->buffer == NULL)
+ Buf_Init(bp, size);
+ else
+ Buf_Reset(bp);
+}
+
void
Buf_Init(Buffer bp, size_t size)
{
diff --git a/buf.h b/buf.h
index 1b56b27..20ea56a 100644
--- a/buf.h
+++ b/buf.h
@@ -106,6 +106,9 @@ extern void Buf_AddChars(Buffer, size_t, const char *);
* Initializes a buffer, to hold approximately init chars.
* Set init to 0 if you have no idea. */
extern void Buf_Init(Buffer, size_t);
+/* Buf_Reinit(buf, init);
+ * Initializes/reset a static buffer */
+extern void Buf_Reinit(Buffer, size_t);
/* Buf_Destroy(buf);
* Nukes a buffer and all its resources. */
#define Buf_Destroy(bp) ((void)free((bp)->buffer))
diff --git a/compat.c b/compat.c
index 069e52f..fd78d78 100644
--- a/compat.c
+++ b/compat.c
@@ -193,14 +193,12 @@ CompatMake(void *gnp, /* The node to make */
/* copy over what we just did */
gn->built_status = sib->built_status;
- if (gn->built_status != ERROR) {
- /* If the node was built successfully, mark it so,
+ if (gn->built_status == REBUILT) {
+ /* If the node was built successfully,
* update its modification time and timestamp all
* its parents.
* This is to keep its state from affecting that of
* its parent. */
- gn->built_status = REBUILT;
- sib->built_status = REBUILT;
/* This is what Make does and it's actually a good
* thing, as it allows rules like
*
@@
Re: MAKE: redux patch
On Fri, Jan 10, 2020 at 10:52:03PM +0100, Alexander Bluhm wrote:
> On Fri, Jan 10, 2020 at 01:58:47PM +0100, Marc Espie wrote:
> > Bleh, I forgot to synch two patches I already committed. Here's a patch
> > that applies cleanly.
>
> I did run this make through a full regress. It seems that make
> regress in /usr/src/regress/usr.sbin/ldapd/ exits with an error.
>
> http://bluhm.genua.de/regress/results/2020-01-10T14%3A11%3A58Z/logs/usr.sbin/ldapd/make.log
>
> *** Error 1 in /usr/src/regress/usr.sbin/ldapd (Makefile:44 '.END': @[ -f
> /usr/src/regress/usr.sbin/ldapd/obj/ldapd.pid ] && kill $(cat /us...)
>
> I have never seen this problem before.
>
> bluhm
Oh, the test is wrong, and it's now enough to have make complain about it.
Before the patch, errors in .END and .BEGIN were not properly looked at.
See, that -f ... && will be *false* if the file doesn't exist.
The way to this kind of test without failing is
if [ -f ... ]; then ... fi
Index: Makefile
===
RCS file: /cvs/src/regress/usr.sbin/ldapd/Makefile,v
retrieving revision 1.9
diff -u -p -r1.9 Makefile
--- Makefile28 Jun 2018 02:47:55 - 1.9
+++ Makefile11 Jan 2020 11:29:35 -
@@ -39,9 +39,10 @@ bootstrap:
.if ! (make(clean) || make(cleandir) || make(obj))
.END:
- @[ -f ${.OBJDIR}/ldapd.pid ] &&\
+ @if [ -f ${.OBJDIR}/ldapd.pid ]; then \
${SUDO} kill $$(cat ${.OBJDIR}/ldapd.pid) &&\
- rm ${.OBJDIR}/ldapd.pid
+ rm ${.OBJDIR}/ldapd.pid; \
+ fi
.endif
connect: bootstrap
Re: MAKE: some older keywords
More specifically, I'm running with this patch
The specific choice of keywords to deprecate is up for grabs, the
infrastructure for actually being able to error out on these keywords is
probably something I should commit anyhow.
diff --git a/gnode.h b/gnode.h
index 283fead..04e5462 100644
--- a/gnode.h
+++ b/gnode.h
@@ -77,6 +77,8 @@
#define SPECIAL_NOTHING6U /* this is used for things we
* recognize for compatibility but
* don't do anything with... */
+#define SPECIAL_DEPRECATED 7U /* this is an old keyword and it will
+* trigger a fatal error. */
#define SPECIAL_INVISIBLE 8U
#define SPECIAL_JOIN 9U
#define SPECIAL_MADE 11U
diff --git a/parse.c b/parse.c
index dfc2abc..555d2cd 100644
--- a/parse.c
+++ b/parse.c
@@ -184,13 +184,13 @@ static struct {
unsigned int special;
unsigned int special_op;
} specials[] = {
-{ P(NODE_EXEC),SPECIAL_EXEC, OP_EXEC },
+{ P(NODE_EXEC),SPECIAL_DEPRECATED, OP_EXEC },
{ P(NODE_IGNORE), SPECIAL_IGNORE, OP_IGNORE },
-{ P(NODE_INCLUDES),SPECIAL_NOTHING,0 },
-{ P(NODE_INVISIBLE), SPECIAL_INVISIBLE, OP_INVISIBLE },
-{ P(NODE_JOIN),SPECIAL_JOIN, OP_JOIN },
-{ P(NODE_LIBS),SPECIAL_NOTHING,0 },
-{ P(NODE_MADE),SPECIAL_MADE, OP_MADE },
+{ P(NODE_INCLUDES),SPECIAL_DEPRECATED, 0 },
+{ P(NODE_INVISIBLE), SPECIAL_DEPRECATED, OP_INVISIBLE },
+{ P(NODE_JOIN),SPECIAL_DEPRECATED, OP_JOIN },
+{ P(NODE_LIBS),SPECIAL_DEPRECATED, 0 },
+{ P(NODE_MADE),SPECIAL_DEPRECATED, OP_MADE },
{ P(NODE_MAIN),SPECIAL_MAIN, 0 },
{ P(NODE_MAKE),SPECIAL_MAKE, OP_MAKE },
{ P(NODE_MAKEFLAGS), SPECIAL_MFLAGS, 0 },
@@ -198,7 +198,7 @@ static struct {
{ P(NODE_NOTMAIN), SPECIAL_NOTMAIN,OP_NOTMAIN },
{ P(NODE_NOTPARALLEL), SPECIAL_NOTPARALLEL,0 },
{ P(NODE_NO_PARALLEL), SPECIAL_NOTPARALLEL,0 },
-{ P(NODE_NULL),SPECIAL_NOTHING,0 },
+{ P(NODE_NULL),SPECIAL_DEPRECATED, 0 },
{ P(NODE_OPTIONAL),SPECIAL_OPTIONAL, OP_OPTIONAL },
{ P(NODE_ORDER), SPECIAL_ORDER, 0 },
{ P(NODE_PARALLEL),SPECIAL_PARALLEL, 0 },
@@ -418,6 +418,11 @@ ParseDoSrc(
const char *esrc)
{
GNode *gn = Targ_FindNodei(src, esrc, TARG_CREATE);
+ if (gn->special == SPECIAL_DEPRECATED) {
+ Parse_Error(PARSE_FATAL, "Deprecated keyword found %s\n",
+ gn->name);
+ return;
+ }
if (gn->special_op) {
Array_ForEach(targets, ParseDoSpecial, gn->special_op);
return;
@@ -702,6 +707,13 @@ handle_special_targets(Lst paths)
for (i = 0; i < gtargets.n; i++) {
type = gtargets.a[i]->special;
+ if (type == SPECIAL_DEPRECATED) {
+ Parse_Error(PARSE_FATAL,
+ "Deprecated keyword found %s\n",
+ gtargets.a[i]->name);
+ specType = SPECIAL_ERROR;
+ return 0;
+ }
if (type == SPECIAL_PATH) {
seen_path++;
Lst_AtEnd(paths, find_suffix_path(gtargets.a[i]));
Re: MAKE: redux patch
On Thu, Jan 09, 2020 at 01:09:59PM +0100, Marc Espie wrote:
> So my development branch is getting a bit too far
> ahead compared to what's committed.
>
> Here's a big diff to test. Comments as to what's going on
> and the changes this contains:
>
> - buffer changes: add support for "permanent static buffers"
> that don't get reinit'd all the time (that's a mostly trivial
> change)
>
> - parser change: SPECIAL_TARGET and SPECIAL_SOURCE are not
> really needed (figured out in netbsd)
> - (related): create the special nodes once in a simpler way
>
> - a bit of reorg: take the code for expand_children out
> of suff.c proper, as this file is always fairly unwieldy.
> This only requires Link_Parent, and find_best_path, which
> are suff-independent.
>
> - engine change: stop mixing parallel and compat so much.
> In particular, don't run Make_Update when in compat mode (which
> would happen because handle_all_signals goes into the main loop)
>
> - (related): abstract the engine running into operations in
> 'enginechoice.c' so that it's perfectly clear what part is compatMake
> and what part is not. Have it report out_of_date and errors in
> a simple way. Don't mix up .BEGIN/.END handling.
>
> - (related): job.c determines whether to use the expensive heuristics
> or not depending on how many jobs we run.
>
> - jobrunner cleanup: we have no need for maxjobs, it's enough to move
> stuff around from available into running or error... do the move to
> error later so that everything is in the same location.
>
> - with the above changes: the special case of running_one_job is no
> longer needed at all.
>
> - feature coming from bmake: treat .BEGIN/.END more as normal targets.
> More specifically, just invoke the engine on them if they exist.
> This makes it possible to have
> .BEGIN: somethingelse
> which does make a lot of sense, actually.
>
> I would really need this to go in so that I can keep pushing forward.
>
> I think I might take out the "old" parallel engine (which is somewhat
> broken, and frankly, I don't get how it can work) because modifying
> compat to actually queue stuff and build it is a distinct possibility
> now. e.g., having an actual parallel engine that works.
Bleh, I forgot to synch two patches I already committed. Here's a patch
that applies cleanly.
diff --git a/Makefile b/Makefile
index 0cd84fc..90747de 100644
--- a/Makefile
+++ b/Makefile
@@ -16,8 +16,8 @@ CFLAGS+=${CDEFS}
HOSTCFLAGS+=${CDEFS}
SRCS= arch.c buf.c cmd_exec.c compat.c cond.c dir.c direxpand.c dump.c \
- engine.c \
- error.c for.c init.c job.c lowparse.c main.c make.c memory.c parse.c \
+ engine.c enginechoice.c error.c expandchildren.c \
+ for.c init.c job.c lowparse.c main.c make.c memory.c parse.c \
parsevar.c str.c stats.c suff.c targ.c targequiv.c timestamp.c \
var.c varmodifiers.c varname.c
diff --git a/arch.c b/arch.c
index 85e8e7e..02c18b5 100644
--- a/arch.c
+++ b/arch.c
@@ -195,11 +195,10 @@ bool
Arch_ParseArchive(const char **line, Lst nodes, SymTable *ctxt)
{
bool result;
- BUFFER expand;
+ static BUFFER expand;
- Buf_Init(, MAKE_BSIZE);
+ Buf_Reinit(, MAKE_BSIZE);
result = parse_archive(, line, nodes, ctxt);
- Buf_Destroy();
return result;
}
diff --git a/buf.c b/buf.c
index 74cbfe8..931bd7e 100644
--- a/buf.c
+++ b/buf.c
@@ -153,6 +153,15 @@ Buf_printf(Buffer bp, const char *fmt, ...)
bp->inPtr += n;
}
+void
+Buf_Reinit(Buffer bp, size_t size)
+{
+ if (bp->buffer == NULL)
+ Buf_Init(bp, size);
+ else
+ Buf_Reset(bp);
+}
+
void
Buf_Init(Buffer bp, size_t size)
{
diff --git a/buf.h b/buf.h
index 1b56b27..20ea56a 100644
--- a/buf.h
+++ b/buf.h
@@ -106,6 +106,9 @@ extern void Buf_AddChars(Buffer, size_t, const char *);
* Initializes a buffer, to hold approximately init chars.
* Set init to 0 if you have no idea. */
extern void Buf_Init(Buffer, size_t);
+/* Buf_Reinit(buf, init);
+ * Initializes/reset a static buffer */
+extern void Buf_Reinit(Buffer, size_t);
/* Buf_Destroy(buf);
* Nukes a buffer and all its resources. */
#define Buf_Destroy(bp) ((void)free((bp)->buffer))
diff --git a/compat.c b/compat.c
index 7ecb75f..fd78d78 100644
--- a/compat.c
+++ b/compat.c
@@ -193,14 +193,12 @@ CompatMake(void *gnp, /* The node to make */
/* copy over what we just did */
gn->built_status = sib->built_status;
- if (gn->built_status != ERROR) {
- /* If the node was built successfully, mark it so,
+ if (gn->built_status == REBUILT) {
+ /* If the node was built successfully,
* update it
MAKE: simplify compat handling
The convoluted logic that resets must_make does not make any sense to me.
It's just as simple to set built_status to ABORTED directly.
Note that in the compat make case, there are two instances of using
must_make left, one in arch.c, which I have yet to understand, and one
in node_failure/list_parents.
diff --git a/compat.c b/compat.c
index fd78d78..9173f44 100644
--- a/compat.c
+++ b/compat.c
@@ -116,9 +116,7 @@ CompatMake(void *gnp, /* The node to make */
* transformations the suffix module thinks are necessary.
* Once that's done, we can descend and make all our children.
* If any of them has an error but the -k flag was given,
-* our 'must_make' field will be set false again. This is our
-* signal to not attempt to do anything but abort our
-* parent as well. */
+* we will abort. */
gn->must_make = true;
gn->built_status = BUILDING;
/* note that, in case we have siblings, we only check all
@@ -132,10 +130,9 @@ CompatMake(void *gnp, /* The node to make */
sib = sib->sibling;
} while (sib != gn);
- if (!gn->must_make) {
+ if (gn->built_status == ABORTED) {
Error("Build for %s aborted", gn->name);
- gn->built_status = ABORTED;
- pgn->must_make = false;
+ pgn->built_status = ABORTED;
return;
}
@@ -233,7 +230,7 @@ CompatMake(void *gnp, /* The node to make */
Make_TimeStamp(pgn, gn);
}
} else if (keepgoing)
- pgn->must_make = false;
+ pgn->built_status = ABORTED;
else {
print_errors();
exit(1);
@@ -242,12 +239,12 @@ CompatMake(void *gnp, /* The node to make */
case ERROR:
/* Already had an error when making this beastie. Tell the
* parent to abort. */
- pgn->must_make = false;
+ pgn->built_status = ABORTED;
break;
case BUILDING:
Error("Graph cycles through %s", gn->name);
gn->built_status = ERROR;
- pgn->must_make = false;
+ pgn->built_status = ABORTED;
break;
case REBUILT:
if ((gn->type & OP_EXEC) == 0) {
MAKE: remove redundant test
The only way errorJobs ever gets filled is by engine.c
around line 680 with a snippet that reads
if (!keepgoing) {
if (!silent)
printf("\n");
job->next = errorJobs;
errorJobs = job;
/* XXX don't free the command */
return;
}
Index: job.c
===
RCS file: /cvs/src/usr.bin/make/job.c,v
retrieving revision 1.144
diff -u -p -r1.144 job.c
--- job.c 31 Dec 2019 13:59:14 - 1.144
+++ job.c 4 Jan 2020 12:14:50 -
@@ -546,8 +547,7 @@ postprocess_job(Job *job)
} else if (job->exit_type != JOB_EXIT_OKAY && keepgoing)
free(job);
- if (errorJobs != NULL && !keepgoing &&
- aborting != ABORT_INTERRUPT)
+ if (errorJobs != NULL && aborting != ABORT_INTERRUPT)
aborting = ABORT_ERROR;
if (aborting == ABORT_ERROR && DEBUG(QUICKDEATH))
MAKE: plug memory leak
If we look at job_handle_status, we see there's one case where it doesn't place the job on the errorJob list, but doesn't try to reclaim the memory. Index: job.c === RCS file: /cvs/src/usr.bin/make/job.c,v retrieving revision 1.143 diff -u -p -r1.143 job.c --- job.c 30 Dec 2019 11:01:16 - 1.143 +++ job.c 31 Dec 2019 11:46:57 - @@ -543,7 +544,8 @@ postprocess_job(Job *job) job->node->built_status = REBUILT; Make_Update(job->node); free(job); - } + } else if (job->exit_type != JOB_EXIT_OKAY && keepgoing) + free(job); if (errorJobs != NULL && !keepgoing && aborting != ABORT_INTERRUPT)
MAKE: have the "expensive job" logic perform better
So, a job is the series of shell commands run by make
a:
cmd1
expensive_cmd2
cmd3
so the granularity of expensive should be at the command level,
not the job level.
So, instead of running the code that may start new jobs at
the end of remove_job, let's run it after each change of state.
This should let parallel make perform slightly better, by firing
up jobs sooner in case of several targets with multiple commands.
Index: job.c
===
RCS file: /cvs/src/usr.bin/make/job.c,v
retrieving revision 1.143
diff -u -p -r1.143 job.c
--- job.c 30 Dec 2019 11:01:16 - 1.143
+++ job.c 31 Dec 2019 10:00:39 -
@@ -148,6 +148,7 @@ static void may_continue_job(Job *);
static void continue_job(Job *);
static Job *reap_finished_job(pid_t);
static bool reap_jobs(void);
+static void may_continue_heldback_jobs();
static void loop_handle_running_jobs(void);
static bool expensive_job(Job *);
@@ -746,9 +747,14 @@ remove_job(Job *job)
{
nJobs--;
postprocess_job(job);
+}
+
+static void
+may_continue_heldback_jobs()
+{
while (!no_new_jobs) {
if (heldJobs != NULL) {
- job = heldJobs;
+ Job *job = heldJobs;
heldJobs = heldJobs->next;
if (DEBUG(EXPENSIVE))
fprintf(stderr, "[%ld] cheap -> release %s\n",
@@ -803,6 +809,7 @@ reap_jobs(void)
job_handle_status(job, status);
determine_job_next_step(job);
}
+ may_continue_heldback_jobs();
}
/* sanity check, should not happen */
if (pid == -1 && errno == ECHILD && runningJobs != NULL)
Re: Add -R alias to -r for scp(1)
On Thu, Jan 02, 2020 at 11:19:30AM +0100, Ingo Schwarze wrote: > Hi Kurt, > > Kurt Mosiejczuk wrote on Wed, Jan 01, 2020 at 08:21:04PM -0500: > > > cp(1) uses -R for recursive copy. scp(1) uses -r. This diff adds -R > > as an alias for -r to scp(1) for those assuming consistency with cp(1). > > even if cp -R and scp -r did the same thing - which, if i understand > tedu@ correctly, is not the case - i would still dislike introducing > an alias. > > Aliases make documentation longer for no benefit and sometimes > also cause confusion. They also block namespace for no benefit > (or rather, option space in this case). > > While it is nice when different commands can use the same options > for similar purposes, that is not always possible. And even when > it would have been possible when designing the tools in the past, > if a different decision was made back then, remaining consistent > with previous versions of the same command is usually worth more > than symmetry with a different command. > > Sometimes, aliases are needed because different other operating > systems use different names and we want to be compatible with both. > But whenever they can be avoided, they should. > > When working on documentation, i'm actively trying to deprecate > aliases where that is possible. > > Yours, > Ingo Contrary point: I'm with Kurt on that one. Very often, I do mix up -r and -R, and I do hate it. Especially since our cp *does* have a -r which is subtly different. Not having -R in scp means you can do all kinds of fuckups. Once in three times, I type scp -R and go "oh fuck" when it doesn't work. And if I use scp enough, I'm also likely to use cp -r by mistake. Are we likely to actually remove cp -r so the second mistake doesn't happen ?
PATCH: fix race condition in binutils-2.17 build
We don't normally hit this thanks to the "expensive job" heuristics
(meaning that we don't start the second target while the first is
building), but still there is a race, and if for whatever reason we
remove that heuristics we hit it fairly hard.
The patch is fairly trivial, just run both targets separately.
Example of a crash and analysis follows.
Note that the problem only occurs in binutils-2.17. The former versions
did have one less level of indirection in their Makefiles.
Index: Makefile.bsd-wrapper
===
RCS file: /cvs/src/gnu/usr.bin/binutils-2.17/Makefile.bsd-wrapper,v
retrieving revision 1.19
diff -u -p -r1.19 Makefile.bsd-wrapper
--- Makefile.bsd-wrapper21 Dec 2019 21:40:00 - 1.19
+++ Makefile.bsd-wrapper2 Jan 2020 10:13:01 -
@@ -64,6 +64,7 @@ CONFIGTARGET+=--without-gnu-as
.endif
all: config.status
+.for t in all info
SUBDIRS='${SUBDIRS}' \
CONFIGURE_HOST_MODULES='${CONFIGURE_HOST_MODULES}' \
${MAKE} ${XCFLAGS} \
@@ -74,7 +75,8 @@ all: config.status
BSDSRCDIR=${BSDSRCDIR} \
ALL_MODULES="${ALL_MODULES}" \
ALL_HOST_MODULES='${ALL_HOST_MODULES}' \
- INFO_HOST_MODULES='${INFO_HOST_MODULES}' all info
+ INFO_HOST_MODULES='${INFO_HOST_MODULES}' $t
+.endfor
.include
@@ -115,6 +117,7 @@ config.status: do-config
# Need to pass SUBDIRS because of install-info
install: maninstall
+.for t in install install-info
${INSTALL} -d -o ${BINOWN} -g ${BINGRP} \
${DESTDIR}${PREFIX}/libdata/ldscripts
SUBDIRS='${INST_SUBDIRS}' ${MAKE} DESTDIR='${DESTDIR}' \
@@ -126,7 +129,8 @@ install: maninstall
INSTALL_PROGRAM='${INSTALL} -c ${INSTALL_STRIP} -o ${BINOWN} -g
${BINGRP} -m ${BINMODE}' \
INSTALL_DATA='${INSTALL} -c -o ${DOCOWN} -g ${DOCGRP} -m
${NONBINMODE}' \
INSTALL_INFO_HOST_MODULES='${INSTALL_INFO_HOST_MODULES}' \
- install install-info
+ $t
+.endfor
.if ${LINKER_VERSION:L} == "bfd"
rm -f ${DESTDIR}${PREFIX}/bin/ld
ln ${DESTDIR}${PREFIX}/bin/ld.bfd ${DESTDIR}${PREFIX}/bin/ld
Tail of a crashing build:
checking for LC_MESSAGES... (cached) yes
checking for LC_MESSAGES... (cached) yes
checking whether NLS is requested... no
checking whether NLS is requested... no
updating cache ./config.cache
updating cache ./config.cache
creating ./config.status
creating ./config.status
mv: rename conftest.tail to conftest.vals: No such file or directory
mv: conftest.tail: No such file or directory
mv: conftest.tail: No such file or directory
mv: conftest.tail: No such file or directory
mv: conftest.tail: No such file or directory
mv: conftest.tail: No such file or directory
mv: conftest.tail: No such file or directory
mv: conftest.tail: No such file or directory
mv: conftest.tail: No such file or directory
mv: conftest.tail: No such file or directory
mv: conftest.tail: No such file or directory
creating Makefile
creating Makefile
./config.status[812]: syntax error: `fi' unexpected
./config.status[812]: syntax error: `fi' unexpected
*** Error 1 in gnu/usr.bin/binutils-2.17/obj (Makefile:5406 'configure-intl':
@r=`${PWDCMD-pwd}`; export r; s=`cd /usr/src/gnu/usr.bin/binu...)
*** Error 1 in gnu/usr.bin/binutils-2.17/obj (Makefile:5406 'configure-intl':
@r=`${PWDCMD-pwd}`; export r; s=`cd /usr/src/gnu/usr.bin/binu...)
*** Error 2 in target 'all'
*** Error 2 in gnu/usr.bin/binutils-2.17/obj (Makefile:747 'do-info':
@r=`${PWDCMD-pwd}`; export r; s=`cd /usr/src/gnu/usr.bin/binutils-2.1...)
*** Error 2 (Makefile:634 'all': @r=`${PWDCMD-pwd}`; export r; s=`cd
/usr/src/gnu/usr.bin/binutils-2.17; ${PWDCMD-pwd}`; export s; if [ -f...)
*** Error 2 in gnu/usr.bin/binutils-2.17 (Makefile.bsd-wrapper:77 'all')
You'll notice the duplicate lines. It's actually libintl running configure
twice at the same time. Initially, I thought "oh let's configure libintl
up-front". Unfortunately this is not enough, and leads to a different crash.
Now, for the generated Makefile. In binutils prior to 2.17, both all and
info go straight info building, so they're not affected.
binutils-2.17 has the following snippets:
all:
@: $(MAKE); $(unstage)
@r=`${PWD_COMMAND}`; export r; \
s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
if [ -f stage_last ]; then \
$(MAKE) $(TARGET_FLAGS_TO_PASS) all-host all-target; \
else \
$(MAKE) $(RECURSE_FLAGS_TO_PASS) all-host all-target; \
fi
(the : $(MAKE) is the fun way automake/autoconf disables maintenance lines)
and
info: do-info
do-info:
@: $(MAKE); $(unstage)
@r=`${PWD_COMMAND}`; export r; \
s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
$(MAKE) $(RECURSE_FLAGS_TO_PASS) info-host \
info-target
so, if they're run at the same time, they will *both* recurse and run
a separate make...
looking a bit further,
Re: cdio(1): remove CDDB support
On Sat, Dec 28, 2019 at 07:48:45PM -0500, Bryan Steele wrote: > With FreeDB announcing[0] that the service will be shutting down as of > March 31st of 2020, and the only other alternative (MusicBrainz) already > having shutdown their freedb/cddb gateway in favour of their own API > early this year, it likely makes sense to remove support from cdio(1). > > CDDB is used to retrieve music CD metadata over the Internet, e.g: > Artist and Track names. > > I left in support for the "cdid" command as it may be useful for > archival(?) purposes, if not that can go too. > > Cc: espie@ as he wrote this code, & maybe he still has music on CDs. How about we wait until the service actually shuts down ? Who knows, maybe someone else will pick up ? It's fairly sad, as this means you would have (again) to annotate mp3s you make from your own CDs by hand.
MAKE: fix -q flag
-q is just plain broken in compat mode, it's easy to fix though.
The second issue is that, if a Makefile as an .END target, that one
will always be run, even in query mode, contrary to .BEGIN.
So let's fix that as well.
Fairly obvious patch
Index: compat.c
===
RCS file: /cvs/src/usr.bin/make/compat.c,v
retrieving revision 1.88
diff -u -p -r1.88 compat.c
--- compat.c21 Dec 2019 15:29:25 - 1.88
+++ compat.c5 Jan 2020 13:43:32 -
@@ -266,11 +266,12 @@ CompatMake(void *gnp, /* The node to mak
}
}
-void
+bool
Compat_Run(Lst targs) /* List of target nodes to re-create */
{
GNode *gn = NULL; /* Current root target */
int errors; /* Number of targets not built due to errors */
+ boolout_of_date = false;
/* For each entry in the list of targets to create, call CompatMake on
* it to create the thing. CompatMake will leave the 'built_status'
@@ -291,11 +292,15 @@ Compat_Run(Lst targs) /* List of target
else if (gn->built_status == ABORTED) {
printf("`%s' not remade because of errors.\n",
gn->name);
+ out_of_date = true;
errors++;
+ } else {
+ out_of_date = true;
}
}
/* If the user has defined a .END target, run its commands. */
- if (errors == 0)
+ if (errors == 0 && !queryFlag)
run_gnode(end_node);
+ return out_of_date;
}
Index: compat.h
===
RCS file: /cvs/src/usr.bin/make/compat.h,v
retrieving revision 1.3
diff -u -p -r1.3 compat.h
--- compat.h19 Jul 2010 19:46:43 - 1.3
+++ compat.h5 Jan 2020 13:43:32 -
@@ -35,8 +35,9 @@
* - friendly variable substitution.
*/
-/* Compat_Run(to_create);
- * Run the actual make engine, to create targets that need to. */
-extern void Compat_Run(Lst);
+/* out_of_date = Compat_Run(to_create);
+ * Run the actual make engine, to create targets that need to,
+ * return true if any target is out of date. */
+extern bool Compat_Run(Lst);
#endif
Index: main.c
===
RCS file: /cvs/src/usr.bin/make/main.c,v
retrieving revision 1.123
diff -u -p -r1.123 main.c
--- main.c 22 Apr 2019 18:32:09 - 1.123
+++ main.c 5 Jan 2020 13:43:32 -
@@ -804,7 +804,7 @@ main(int argc, char **argv)
if (compatMake)
/* Compat_Init will take care of creating all the
* targets as well as initializing the module. */
- Compat_Run();
+ outOfDate = Compat_Run();
else {
/* Traverse the graph, checking on all the targets. */
outOfDate = Make_Run();
Index: make.c
===
RCS file: /cvs/src/usr.bin/make/make.c,v
retrieving revision 1.76
diff -u -p -r1.76 make.c
--- make.c 21 Dec 2019 15:31:54 - 1.76
+++ make.c 5 Jan 2020 13:43:32 -
@@ -572,7 +572,8 @@ Make_Run(Lst targs) /* the initial list
(void)MakeStartJobs();
}
- problem = Job_Finish();
+ if (!queryFlag)
+ problem = Job_Finish();
/*
* Print the final status of each target. E.g. if it wasn't made
Re: make patch: prefer standard behavior
On Thu, Apr 09, 2020 at 06:33:25PM +0200, Marc Espie wrote:
> Our make (bsd make) has "alternate" dependency operators :: and !
>
> Twenty years ago, we made it so that ambiguous constructs like
> File::Find.3p: File/Find.pm
>
> would no longer be ambiguous thanks to the added space.
>
> Now, I believe that ! should be trumped by : if both are present in the
> same line without intervening spaces.
>
> This means that standard Makefile lines won't be interpreted differently
> if run under our make.
>
> This is what this patch achieves.
>
> okay? objections ?
>
> Index: parse.c
> ===
> RCS file: /cvs/src/usr.bin/make/parse.c,v
> retrieving revision 1.132
> diff -u -p -r1.132 parse.c
> --- parse.c 26 Jan 2020 12:41:21 - 1.132
> +++ parse.c 9 Apr 2020 16:30:17 -
> @@ -598,7 +598,10 @@ found_delimiter(const char *s)
> do {
> p += strcspn(p, "!:");
> if (*p == '\0')
> - break;
> + break;
> + /* always prefer : to ! if not space separated */
> + if (*p == ':' && *s == '!')
> + return false;
> p++;
> } while (*p != '\0' && !ISSPACE(*p));
>
>
>
Ping again. I know this is somewhat esoteric, but this actually came up on
ports, where go distfiles do have ! in their names.
Very specifically, we have support for ! as a bsd extension (see the manpage,
it forces re-execution).
This does mean that some standard makefiles on other systems will behave
strangely on OpenBSD.
However, ambiguous lines are somewhat unlikely. What this does is extend
the rule that arbitrates ambiguities.
- if an operator is followed by a space, it will be the operator. So in
a line like
a! somedependencywitha:
then ! is the make operator
- if there is no space, we will always choose the standard operator, so
some!go!module:
will recognize : as the default operator.
The ! BSD make semantics is rarely used, but I've seen it a few times, and
I think this patch is actually fairly safe. But I need eyes and okays...
make patch: prefer standard behavior
Our make (bsd make) has "alternate" dependency operators :: and !
Twenty years ago, we made it so that ambiguous constructs like
File::Find.3p: File/Find.pm
would no longer be ambiguous thanks to the added space.
Now, I believe that ! should be trumped by : if both are present in the
same line without intervening spaces.
This means that standard Makefile lines won't be interpreted differently
if run under our make.
This is what this patch achieves.
okay? objections ?
Index: parse.c
===
RCS file: /cvs/src/usr.bin/make/parse.c,v
retrieving revision 1.132
diff -u -p -r1.132 parse.c
--- parse.c 26 Jan 2020 12:41:21 - 1.132
+++ parse.c 9 Apr 2020 16:30:17 -
@@ -598,7 +598,10 @@ found_delimiter(const char *s)
do {
p += strcspn(p, "!:");
if (*p == '\0')
- break;
+ break;
+ /* always prefer : to ! if not space separated */
+ if (*p == ':' && *s == '!')
+ return false;
p++;
} while (*p != '\0' && !ISSPACE(*p));
Re: find(1) -exec + and ARG_MAX
On Thu, Apr 09, 2020 at 02:44:14PM +0200, Jeremie Courreges-Anglas wrote:
> On Thu, Apr 09 2020, Jeremie Courreges-Anglas wrote:
> > find(1) uses ARG_MAX to compute the maximum space it can pass to
> > execve(2). This doesn't fly if userland and the kernel don't agree, as
> > noticed by some after the recent ARG_MAX bump.
> >
> > --8<--
> > ritchie /usr/src/usr.bin/find$ obj/find /usr/src/ -type f -exec true {} +
> > find: true: Argument list too long
> > find: true: Argument list too long
> > find: true: Argument list too long
> > ^C
> > -->8--
> >
> > While having the kernel and userland out of sync is not a good idea,
> > making find(1) more robust by using sysconf(3) is easy. This is what
> > xargs(1) already does.
> >
> > ok?
>
> Committed, thanks.
>
> > PS: a followup diff will take into account the space taken by the
> > environment
>
> Right now we don't account for the space used by the environment.
> We get lucky either because of the 5000 max args limit or because
> environment size usually fits in the 4096 bytes safety net.
>
> The diff below uses the same approach as xargs(1).
> While here, tweak the message in case of (unlikely) sysconf(3) failure.
You drop a bit of the comment in find, specifically the 4K stuff.
I would probably restore it.
Note that there is an explanation for the overflow in the kernel code:
MAXINTERP + MAXPATHLEN
exec does not reallocate args for scripts, but requires the extra space to
set things up.
Re: Mention /etc/examples/ in those config files manpages + FILES short description
I do think newcomers will tend to miss examples, even if /etc/examples is somewhat documented elsewhere. Also, queries to mandocdb will probably benefit... say "what commands do have an example configuration file ?" kind of what you should be able to ask with advanced queries.
Re: Mouse movement speed
I haven't used wsmoused in ages, but I tend to think ideas like (by default) scaling to the font size have great merit. There should be a debugging toggle (maybe) to get raw values. It should work out-of-the-box preferably. ;)
tar confused error messages
If tar can't create intermediate directories due to permission
issues, the resulting message is confusing:
./tar xf gcc.tar gcc-8.3.0/include/obstack.h
tar: Unable to create gcc-8.3.0/include/obstack.h: No such file or directory
(here I have gcc-8.3.0 owned by root and no permissions)
The following patch changes this to:
./tar xf gcc.tar gcc-8.3.0/include/obstack.h
tar: Unable to create gcc-8.3.0/include: Permission denied
tar: Unable to create gcc-8.3.0/include/obstack.h: No such file or directory
okay ? or a better way to do this ?
Index: extern.h
===
RCS file: /cvs/src/bin/pax/extern.h,v
retrieving revision 1.59
diff -u -p -r1.59 extern.h
--- extern.h13 Sep 2018 12:33:43 - 1.59
+++ extern.h22 Mar 2020 10:39:53 -
@@ -128,7 +128,7 @@ int cross_lnk(ARCHD *);
int chk_same(ARCHD *);
int node_creat(ARCHD *);
int unlnk_exist(char *, int);
-int chk_path(char *, uid_t, gid_t);
+int chk_path(char *, uid_t, gid_t, int);
void set_ftime(const char *, const struct timespec *,
const struct timespec *, int);
void fset_ftime(const char *, int, const struct timespec *,
Index: file_subs.c
===
RCS file: /cvs/src/bin/pax/file_subs.c,v
retrieving revision 1.54
diff -u -p -r1.54 file_subs.c
--- file_subs.c 28 Jun 2019 13:34:59 - 1.54
+++ file_subs.c 22 Mar 2020 10:39:53 -
@@ -102,7 +102,7 @@ file_creat(ARCHD *arcn)
file_mode)) >= 0)
break;
oerrno = errno;
- if (nodirs ||
chk_path(arcn->name,arcn->sb.st_uid,arcn->sb.st_gid) < 0) {
+ if (nodirs ||
chk_path(arcn->name,arcn->sb.st_uid,arcn->sb.st_gid, 0) < 0) {
syswarn(1, oerrno, "Unable to create %s", arcn->name);
return(-1);
}
@@ -316,7 +316,7 @@ mk_link(char *to, struct stat *to_sb, ch
if (linkat(AT_FDCWD, to, AT_FDCWD, from, 0) == 0)
break;
oerrno = errno;
- if (!nodirs && chk_path(from, to_sb->st_uid, to_sb->st_gid) ==
0)
+ if (!nodirs && chk_path(from, to_sb->st_uid, to_sb->st_gid,
ign) == 0)
continue;
if (!ign) {
syswarn(1, oerrno, "Could not link to %s from %s", to,
@@ -458,7 +458,7 @@ badlink:
if (++pass <= 1)
continue;
- if (nodirs || chk_path(nm,arcn->sb.st_uid,arcn->sb.st_gid) < 0)
{
+ if (nodirs || chk_path(nm,arcn->sb.st_uid,arcn->sb.st_gid, 0) <
0) {
syswarn(1, oerrno, "Could not create: %s", nm);
return(-1);
}
@@ -590,7 +590,7 @@ unlnk_exist(char *name, int type)
*/
int
-chk_path(char *name, uid_t st_uid, gid_t st_gid)
+chk_path(char *name, uid_t st_uid, gid_t st_gid, int ign)
{
char *spt = name;
char *next;
@@ -643,6 +643,8 @@ chk_path(char *name, uid_t st_uid, gid_t
* needed directory and continue on
*/
if (mkdir(name, S_IRWXU | S_IRWXG | S_IRWXO) == -1) {
+ if (!ign)
+ syswarn(1, errno, "Unable to create %s", name);
*spt = '/';
retval = -1;
break;
Re: tar confused error messages
On Sun, Mar 22, 2020 at 01:09:17PM +0100, Denis Fondras wrote: > On Sun, Mar 22, 2020 at 11:41:55AM +0100, Marc Espie wrote: > > If tar can't create intermediate directories due to permission > > issues, the resulting message is confusing: > > > > ./tar xf gcc.tar gcc-8.3.0/include/obstack.h > > tar: Unable to create gcc-8.3.0/include/obstack.h: No such file or directory > > > > (here I have gcc-8.3.0 owned by root and no permissions) > > > > The following patch changes this to: > > > > ./tar xf gcc.tar gcc-8.3.0/include/obstack.h > > tar: Unable to create gcc-8.3.0/include: Permission denied > > tar: Unable to create gcc-8.3.0/include/obstack.h: No such file or directory > > > > Why not use errno on line 106 of file_subs.c ? > That would yield "Permission denied" on obstack.h and would avoid having 2 > error > messages. ... except that the error is not on obstack.h, but on a specific directory creation.
xorg packaging issue
There's some inconsistency: lists/xserv/mi:./usr/X11R6/share/aclocal/xorg-macros.m4 lists/xshare/mi:./usr/X11R6/lib/pkgconfig/xorg-macros.pc both should be in xshare This does explain the failures of tightvnc on some bulk machines. By default, proot does NOT copy xserv in the chroot, and I believe it's correct.
Re: locale thread support
On Fri, May 22, 2020 at 02:31:38PM +0200, Mark Kettenis wrote: > > Date: Fri, 22 May 2020 13:50:44 +0200 > > From: Marc Espie > > > > I've had to neuter some setlocale() in mlt, since our code is > > definitely NOT thread-safe. No biggie, since we do not have support > > for LC_NUMERIC right now. > > Hmm, setlocale() is explicitly mentionded as not thread-safe in POSIX. >From a security standpoint, is there a "cheap" way to make setlocale abort() instead of trying to do a double free on when running in a race condition ? > > I think we might want the thread-specific functions, namely stuff like > > strtod_l, or sprintf_l and friends. > > That is . which isn't actually standardized. But there is > a de-facto standard set by Apple. Most OSes have an implementation by > now and our libc++ needs it (and uses a stub implementation for now). > The Darwin and/or FreeBSD man pages are probably the best > documentation for these interfaces. > > > Even if they do not do anything specific right now with a locale object, > > at least they would prevent re-entrency issues. > > Yes, implementing these shouldn't be hard. We already have wctype_l() > and iswctype_l() (which are standardized) which do care about the > locale. But everything else probably doesn't so these functions can > be simple wrappers around the non-_l functions that simply ignore the > locale. It's just work but really doesn't require sepcific skills. > I'm sure Ingo will be able to give some constructive feedback once a > diff exists. > > Note that the current issue is a time-bomb, because it is a race, so > > it does only manifest itself as a double free in freegl... > > > > uselocale is fine, but it is not on windows, so highly portable code > > tends to prefer strtod_l... > > Which is funny since strtod_l is even less standard than uselocale() ;). Well, I had a look at the current code, they changed it yet again, so it's probably going to fail in newer, more funny ways.
locale thread support
I've had to neuter some setlocale() in mlt, since our code is definitely NOT thread-safe. No biggie, since we do not have support for LC_NUMERIC right now. I think we might want the thread-specific functions, namely stuff like strtod_l, or sprintf_l and friends. Even if they do not do anything specific right now with a locale object, at least they would prevent re-entrency issues. Note that the current issue is a time-bomb, because it is a race, so it does only manifest itself as a double free in freegl... uselocale is fine, but it is not on windows, so highly portable code tends to prefer strtod_l...
Re: locale thread support
On Fri, May 22, 2020 at 04:02:31PM +0200, Mark Kettenis wrote: > > From: "Todd C. Miller" > > Date: Fri, 22 May 2020 07:23:55 -0600 > > > > On Fri, 22 May 2020 14:57:11 +0200, Marc Espie wrote: > > > > > From a security standpoint, is there a "cheap" way to make setlocale > > > abort() > > > instead of trying to do a double free on when running in a race > > > condition ? > > > > We could use _THREAD_PRIVATE_MUTEX as we do in other parts of libc. > > That might eliminate two threads racing eachoither in setlocale(), but > it wouldn't stop threads that actually access the locale from > use-after-free type bugs. Unless you use the lock there as well. But > that could have a major performance impact. Well, I'd be happy to have an abort() in case a race is detected. As we said, this is 100% unsupported behavior! but it's reasonably hard to detect. Maybe a simple way to poison setlocale is it's called from two different threads in a given process ?... I have zero idea. My point is NOT to make broken code work, but to make it obvious it's broken! because right now, it's a sporadic race... and even though this is complicated, I fear it's a security issue waiting to happen.
WANTLIB problems and possible solution: the libset design
I want to introduce a new mechanism (libsets) to supplement wantlib... Note that the keyword already exists in pkg_add. the story so far: wantlib evolved from lib-depends basically, you tell a given package that it requires some libs, (WANTLIB) and pkg_create will register those libs in the package as a set of @wantlib annotations, thus hardcoding the major/minor of each library. There's some sanity check as well, both during building packages and installing them, which is that all wantlib must be "reachable": looking through all run dependencies (recursively) of the package the libraries must actually exist in a dependency (recursively) of the package or in base/x11. Whenever you update, things must go strictly forward: if the package version changes, that's enough. Otherwise, we do look at version numbers of direct dependencies AND at wantlib version: if things go backward, then we don't update. If things are the same, we don't update either. Altogether this is a "signature" for the package version: - the package version itself - the version of each direct dependency - the version of each wantlib The problem: In case the set of wantlib changes, you have to bump the revision of all dependent packages, otherwise register-plist (or pkg_add) will complain that package signatures are non comparable. This is a problem if a basic gnome package (say gnomeajasper) adds a libfoobar, because suddenly, all users (direct AND indirect) of that package MUST be updated to - include foobar in their WANTLIB - have their REVISION bumped. The new design: The idea behind "libset" is to be able to specify a "set" of wantlib that corresponds to our package, AND to just write WANTLIB wrt that libset for that specific set of libraries. In practice: - the basic package would get a libset declaration @libset mylibset-1.0 liba,libb,libc - direct and indirect users would have +mylibset in their WANTLIB (the specific character used is up for debate... (mylibset) would be nice as well, but it would interfere with everything shell-quoting this would have several effects: - when making the package, pkg_create would scan dependent packages for the libset declaration, and add the corresponding wantlib - the resulting package signature would ALSO include +mylibset-.10 in their signature. For updates, things change slightly - dependencies must be going forward - libsets MUST be going forward (you still need to bump REVISION if you add a whole libset to a package) - wantlib MUST be going forward, BUT there is a bit of libset comparison: if a wantlib is ONLY mentioned in a new version of the libset compared to the old (or vice-versa), then it is no longer an error, and versions are STILL comparable. (*) This is more or less the basic principle. Note that (*) is a bit tricky: this only concerns wantlib which are exclusively part of a given libset... because if the wantlib is also mentioned elsewhere, then it still has to be given properly. As for libset versions proper, we would go with the exact same version scheme as for packages. The reason for this would lie with bsd.port.mk: I think it's likely that each package would declare ONE single libset at most, so there is zero issue in naming the libset exactly like the package, so a libset declaration would be LIBSET = liba libb libc in the case above, it would become LIBSET = liba libb libc libfoobar and lead to @libset gnomeajaspec:liba,libb,libc,libfoobar (note that I've written "lib" everywhere to make things obvious, but as usual, this should rather be LIBSET = a b c foobar) (semantics for multi-packages left up to the astute reader). (even though it's currently redundant and matches pkgnames, I prefer to have a separate namespace for libset, just in case we eventually want to add several libsets for a given package...) I don't think we need to make libset recursive (e.g., have a libset include another libset) and also, I have zero idea how to implement that. As far as I can figure out, to change a libset, you would only need to bump the rev of the basic port and add to its LIBSET declaration. If wanted, this could even be used for base/X11, we would mostly need to have a smallish libset declarations for (say) all the X11 libraries that are commonly used for something.
Re: WANTLIB problems and possible solution: the libset design
On Sun, Oct 11, 2020 at 06:03:55PM +0200, Landry Breuil wrote: > On Sun, Oct 11, 2020 at 05:03:31PM +0200, Marc Espie wrote: > > On Sun, Oct 11, 2020 at 04:53:18PM +0200, Christian Weisgerber wrote: > > > Marc Espie: > > > > > > > The new design: > > > > > > > > The idea behind "libset" is to be able to specify a "set" of wantlib > > > > that > > > > corresponds to our package, AND to just write WANTLIB wrt that libset > > > > for > > > > that specific set of libraries. > > > > > > I'm struggling to understand whether this libset records the libraries > > > a port depends on, the libraries the port provides, or both. > > > > > > Let's say--slightly simplified from reality--we have devel/gettext > > > that provides libintl and depends on iconv from converters/libiconv. > > > What would gettext's LIBSET entry look like? > > > > > > (1) LIBSET = iconv > > > (2) LIBSET = intl > > > (3) LIBSET = intl iconv > > > > > > > > I assume any user of gettext will need both iconv and intl, so > > > > LIBSET = intl iconv > > Hmm, pretty sure some consumers of gettext only have one in their > WANTLIB and not both, in that case those ones shouldnt you the libset to > avoid an extra WANTLIB ? How about you make a full sentence with the correct words so I get what you mean ?
Re: WANTLIB problems and possible solution: the libset design
On Sun, Oct 11, 2020 at 06:03:55PM +0200, Landry Breuil wrote: > On Sun, Oct 11, 2020 at 05:03:31PM +0200, Marc Espie wrote: > > On Sun, Oct 11, 2020 at 04:53:18PM +0200, Christian Weisgerber wrote: > > > Marc Espie: > > > > > > > The new design: > > > > > > > > The idea behind "libset" is to be able to specify a "set" of wantlib > > > > that > > > > corresponds to our package, AND to just write WANTLIB wrt that libset > > > > for > > > > that specific set of libraries. > > > > > > I'm struggling to understand whether this libset records the libraries > > > a port depends on, the libraries the port provides, or both. > > > > > > Let's say--slightly simplified from reality--we have devel/gettext > > > that provides libintl and depends on iconv from converters/libiconv. > > > What would gettext's LIBSET entry look like? > > > > > > (1) LIBSET = iconv > > > (2) LIBSET = intl > > > (3) LIBSET = intl iconv > > > > > > > > I assume any user of gettext will need both iconv and intl, so > > > > LIBSET = intl iconv > > Hmm, pretty sure some consumers of gettext only have one in their > WANTLIB and not both, in that case those ones shouldnt you the libset to > avoid an extra WANTLIB ? > On the off chance I understood your meaning, I think it's better to err on the side of having too many WANTLIB than too few. I fail to see an actual downside to updating when iconv gets updated. gettext will often be updated as well.
Re: WANTLIB problems and possible solution: the libset design
On Sun, Oct 11, 2020 at 04:53:18PM +0200, Christian Weisgerber wrote: > Marc Espie: > > > The new design: > > > > The idea behind "libset" is to be able to specify a "set" of wantlib that > > corresponds to our package, AND to just write WANTLIB wrt that libset for > > that specific set of libraries. > > I'm struggling to understand whether this libset records the libraries > a port depends on, the libraries the port provides, or both. > > Let's say--slightly simplified from reality--we have devel/gettext > that provides libintl and depends on iconv from converters/libiconv. > What would gettext's LIBSET entry look like? > > (1) LIBSET = iconv > (2) LIBSET = intl > (3) LIBSET = intl iconv I assume any user of gettext will need both iconv and intl, so LIBSET = intl iconv
Re: locale thread support
On Tue, May 26, 2020 at 12:13:02AM +0200, Ingo Schwarze wrote: > Hi, > > my impression is there are two totally independent topics in this > thread. > > 1. Whether and how to make things safer, ideally terminating the > program in a controlled manner, if it uses functions that are > not thread-safe in an invalid manner. I'm not addressing that > topic in this mail. > > 2. Whether we want additional non-standard xlocale.h functions > in our libc. > > Regarding topic 2, let me say up front that i do not feel strongly > either way. It seems to me this is mostly a question for porters. > If some functions are widely used - even if non-standard - and help > avoid porting hassle, then sure, let's have them unless they cause > excessive fuss. But the latter does not seem likely here. > > There are a number of functions that seem likely useful to me off > the top of my head, for example: querylocale, nl_langinfo_l, > MB_CUR_MAX_L, wcwidth_l > > In general, i must say i like the whole xlocale.h business not all > that much: in a nutshell, it is doubling the number of half the > functions under the sun, which usually indicates poor design in the > first place, and then the vast majority of these additional functions > are almost useless on any operating system and totally useless on > OpenBSD. Like, i mean, btowc_l(3)? Or even the isdigit_l(3) which > we already have? You must be choking, Mr. Chisnall! I don't think > stuff should be added because it is out there, but only if there is > at least some porting benefit. > > Regarding the FreeBSD headers, i like them even less. There are both > terrible design choices and terrible implementation choices. Regarding > design, it appears that you *must* #include after other > headers - if you include it before, it won't work. Regarding > implementation, there is quite repulsive macro abuse in xlocale/_ctype.h; > but probably that can be unrolled in LibreSSL style. Either way, none > of that hinders providing them if doing so yields benefit. > > See below for a list of functions that i drafted extremely quickly, > so beware of errors and omissions. The point isn't to present a > definite plan. The point is merely to demonstrate the sheer fatness > of the animal and to give a first impression of the degree to which > it stinks and grunts from most of its ends. > > If any of this is needed, do porters already know which functions > are in particularly high demand? Do you have any idea how to find > out which ones are actually useful for porting purposes? Serendipity Today, I looked at an older library for audit purposes. Library is called udns. If you look at the code, there's a lot of apparently useless reinvention, like the code explicitly checks for digits using c >= '0' && c <= '9' patterns (or isprint for that matter). Thinking some more, I realized why: this is library code, so it can't assume it's running under any kind of sensible locale unless it has asserted it itself. In that specific context, isdigit_l and friends make a lot of sense. Either that, or systematic uselocale wrappers for any function that can be called from outside. (yes, I now that isdigit/isspace are not a problem on OpenBSD, but they can be elsewhere!)
Re: userland clock_gettime proof of concept
On Thu, May 28, 2020 at 05:44:31PM +0300, Paul Irofti wrote: > Hi, > > Here is a new iteration of the diff which includes support for MD high > resolution clocks. Currently only implements TSC on amd64. If the > MD function is not defined, it fallsback to the syscall. > > There is the question of the skew fix, but that will be addressed in a > separate kernel diff that will not affect the current diff at all. > > I could not find a way to find on which processor the process is running > on from userland without going through a syscall. If there is one please > let me know. It would make things easier. > > In the meantime I have also gotten positive feedback from various > testers that run this on their main machine. > > Anyway, I think we can decide on the struct name and the auxiliary > vector ID and consider this done. > > Thoughts? > > Paul This appears to work just fine here on my desktop. cpu0: AMD A10-5700 APU with Radeon(tm) HD Graphics, 3417.45 MHz, 15-10-01 cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,POPCNT,AES,XSAVE,AVX,F16C,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,XOP,SKINIT,WDT,FMA4,TCE,NODEID,TBM,TOPEXT,CPCTR,ITSC,BMI1 (other bits of dmesg available on demand)
Re: locale thread support
On Fri, May 29, 2020 at 03:48:41PM +0200, Ingo Schwarze wrote: > Hi Marc, > > do i understand correctly that you intend to say: there are > legitimate reasons to use these xlocale.h functions in portable > software, in particular in portable libraries, so we should probably > have them all, or at least most of them? > > I see your point that using functions like isdigit(3) can (at least > in theory) be dangerous when running on systems that support > exceedingly weird single-byte 8-bit locales. (As opposed to only > supporting one single single-byte locale, which is 7-bit, like we do.) > > I'm not even sure the question matters whether xlocale.h is the best > possible way to avoid such issues. To make support desirable, it > might be sufficient that xlocale.h is one reasonable way and that > some amounts of useful software actually use it. > > Yours, > Ingo I'm starting to believe that a xlocale-like model makes more sense than the current model in a lot of cases. Think about it: a lot of locale goo deals with text processing. NOT input/output, but data processing. It seems obvious to me that at least SOME locale information has to be attached to the data, and so each time you have to process the data, you have to choose the right locale. setlocale() is out in modern multi-threaded programs. both uselocale() and xlocale.h functions are NOT portable to everything. xlocale.h allows you to attach the locale to the data uselocale() requires you to do a back each time you want to process data if you don't want to disturb other things. It's hidden globals all over again. It's no accident that libc++ will use xlocale.h functions if they're around. As every other modern library should.
Re: locale thread support
On Tue, May 26, 2020 at 12:13:02AM +0200, Ingo Schwarze wrote: > Regarding the FreeBSD headers, i like them even less. There are both > terrible design choices and terrible implementation choices. Regarding > design, it appears that you *must* #include after other > headers - if you include it before, it won't work. Regarding > implementation, there is quite repulsive macro abuse in xlocale/_ctype.h; > but probably that can be unrolled in LibreSSL style. Either way, none > of that hinders providing them if doing so yields benefit. Thinking a bit more about that, two points: - we do have headers in sys/ that do require a specific order of inclusion, though that has mostly vanished over the years - starting from a FreeBSD-like structure and making xlocale.h position-independent if we so wish is trivial and not even that much work. Each snippet is neatly tucked into an xlocale/ subdirectory, so including xlocale.h early just requires something like #ifdef _X_LOCALE_H #include #endif inside each header...
official ports vs DEBUG_PACKAGES
In a trace:
> > > #3 0x15e48c95459e in WebVfx::shutdown ()
> > > at /usr/obj/ports/webvfx-1.2.0/webvfx-1.2.0/webvfx/webvfx.cpp:193
Now, this is NOT the default location for WRKOBJDIR, but we are shipping
packages with debug information...
This could be a bit cumbersome, if in order to debug locally, you have
to make patch NO_DEPENDS=Yes, and then you figure out you have to mimic the
"official" setup for ports, which does not even match the default.
There are about 3 solutions to that:
- change the bulk machines to /usr/ports/pobj
- change the ports default to /usr/obj/ports
- have some magic I don't know in ELF handling that would allow to either
tweak the default location/introduce ${WRKOBJDIR} in that debug info.
Re: official ports vs DEBUG_PACKAGES
> - have some magic I don't know in ELF handling that would allow to either
> tweak the default location/introduce ${WRKOBJDIR} in that debug info.
Thinking some more about it, that 3rd option is possibly not that
far-fetched
we do pass most debug-info thru dwz for shrinkage, so there's one tool
that does peek inside dwarf debug and does compaction, I have zero idea how
hard it is to tweak paths as well.
Re: official ports vs DEBUG_PACKAGES
On Sat, May 30, 2020 at 05:10:49PM +0200, Jeremie Courreges-Anglas wrote:
> On Sat, May 30 2020, Marc Espie wrote:
> >> - have some magic I don't know in ELF handling that would allow to either
> >> tweak the default location/introduce ${WRKOBJDIR} in that debug info.
> >
> > Thinking some more about it, that 3rd option is possibly not that
> > far-fetched
> >
> > we do pass most debug-info thru dwz for shrinkage, so there's one tool
> > that does peek inside dwarf debug and does compaction, I have zero idea how
> > hard it is to tweak paths as well.
>
> Take a look at -fdebug-prefix-map.
>
> https://reproducible-builds.org/docs/build-path/
Nice, thanks :)
So just adding this to CFLAGS/CXXFLAGS whenever DEBUG_PACKAGES is set
should be enough
Xwindows keymap weirdness
I occasionnally use setxkbmap 'us(intl)' in order to have diacritics as dead keys. I have LC_CTYPE=en_US.UTF-8 If I type 'c I get a ç in programs like firefox or chrome, BUT I get ć in xterm ? why are things different ? which program is right ? (I have just tried with a test user with nothing configured besides LC_CTYPE=en_US.UTF-8, without which xterm/vim doesn't show proper characters)
Re: Xwindows keymap weirdness
On Mon, Jun 01, 2020 at 03:46:09PM +0200, Matthieu Herrb wrote: > On Mon, Jun 01, 2020 at 03:28:52PM +0200, Stéphane Aulery wrote: > > Hello, > > > > Le 01/06/2020 14:55, Matthieu Herrb a écrit : > > > > > > > > > > > (I have just tried with a test user with nothing configured besides > > > > LC_CTYPE=en_US.UTF-8, without which xterm/vim doesn't show proper > > > > characters) > > > > > > I'm using a real US keyboard with AltGr or the Menu Key (depending on > > > the actual keyboard) set as Compose and typing full compose sequences > > > to get diacritics. ieand so on. > > > > I use a Bépo keyboard but this but > > > > Your experience interests me. I use a Bépo keyboard but I plan to switch to > > a QWERTY + compose keyboard like you do. I hope this will give better > > compatibility between systems and less software config remapping. > > > > I do not see how to configure this in console. > > I'm only using this under X. the OpenBSD console is plain ASCII and > has no support for for UTF8 characters, so no need to enter them. > > To setup the right alt key as compose, you can either: > > - run 'setxkbmap -option compose:ralt' somewhere in your session > startup script > > - create /etc/X11/xorg.conf.d/90-keyboard.conf containing > > --- Cut --- > Section "InputClass" > Identifier "Kbd" > MatchDriver "kbd" > Option "XkbOptions" "compose:ralt" > EndSection > --- Cut --- I used to understand that shit back in xmodmap days. I'll admit I'm completely lost with setxkbmap Along the same lines, how can you simply disable caps lock ?
Re: Xwindows keymap weirdness
On Mon, Jun 01, 2020 at 03:37:45PM -0600, Anthony J. Bentley wrote: > Marc Espie writes: > > > To setup the right alt key as compose, you can either: > > > > > > - run 'setxkbmap -option compose:ralt' somewhere in your session > > > startup script > > > > > > - create /etc/X11/xorg.conf.d/90-keyboard.conf containing > > > > > > --- Cut --- > > > Section "InputClass" > > > Identifier "Kbd" > > > MatchDriver "kbd" > > > Option "XkbOptions" "compose:ralt" > > > EndSection > > > --- Cut --- > > > > I used to understand that shit back in xmodmap days. > > I'll admit I'm completely lost with setxkbmap > > > > Along the same lines, how can you simply disable caps lock ? > > caps:none > > Personally I run with caps:ctrl_modifier (caps becomes ctrl). > > These variants are documented in xkeyboard-config(7) (needs a large > terminal to be readable). Oh wow, this doc is a complete piece of shit. I finally figured it out the relationship between setxkbmap and xkeyboard-config thanks to the lines you gave. It would be sooo enlightening to have a few EXAMPLES in setxkbmap(1) that does explain how xkeyboard-config(7) actually works. Last time I looked, I thought I need to create my own description that would be compiled through xkbcomp(1), that's how misleading this so called "documentation" is. Thank you Anthony!
