Re: svn commit: r368736 - head/tools/tools/git/hooks

[Alan Somers]

On Mon, Dec 28, 2020 at 3:29 PM Ed Maste  wrote:

> On Mon, 28 Dec 2020 at 14:34, Alan Somers  wrote:
> >
> > On Thu, Dec 17, 2020 at 12:58 PM Ed Maste  wrote:
> >>
> >> Author: emaste
> >> Date: Thu Dec 17 19:58:29 2020
> >> New Revision: 368736
> >> URL: https://svnweb.freebsd.org/changeset/base/368736
> >>
> >> Log:
> >>   Add initial version of git commit message preparation hook
> >>
> > Thanks!  But how can we use it?  Is there a "git config" incantation
> that will use this file?
>
> Just copy it into .git/hooks/. Can also fetch the latest version
> directly from cgit; lwhsu added this to
> https://github.com/bsdimp/freebsd-git-docs/blob/main/URLs.md:
>
> fetch
> https://cgit.freebsd.org/src/plain/tools/tools/git/hooks/prepare-commit-msg
> -o
> 
> .git/hooks
> chmod 755 .git/hooks/prepare-commit-msg
>
> I'd like us to end up with a setup-repo.sh or such that will offer to
> take care of all of these things.
>
> >Also, this seems too complicated.  Git already has a "config.template"
> configuration variable.   Could we just use that instead, or does it not
> work for some reason?
>
> It does work, although it's a bit awkward. For one thing, the provided
> template is placed at the top of the message, followed by the default
> message, but our addition is really commented-out information and fits
> best in the middle of the existing text - after the "Please enter
> the..." and before the list of changes and such. With config.template
> we'd end up with something like:
>
> 
>
> # PR:   
> # ...
> # Differential Revision:
> #
> # Please enter the commit message for your changes. Lines starting
> # with '#' will be ignored, and an empty message aborts the commit.
> #
> # ...
> 
>
> Additionally we might want to make some more programmatic changes in
> the default message (depending on the resolution of some open
> questions). For example we could strip the "MFC after" line that's
> copied from the original commit, when preparing the commit message for
> a MFC/cherry-pick.
>

Thanks for the explanation.  It all makes sense, and the commit template
works.
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r366697 - head/usr.bin/xinstall

[Alan Somers]

BTW, I have a WIP patch to xinstall to make it use copy_file_range.  The
patch works, but I never wrote a fallback path in case copy_file_range
fails for some reason.  Alex, would you be interested to finish the patch?
-Alan

On Wed, Oct 14, 2020 at 7:35 AM Alexander Richardson <
arichard...@freebsd.org> wrote:

> On Wed, 14 Oct 2020 at 14:29, Mateusz Guzik  wrote:
> >
> > This should use copy_file_range (also available on Linux).
> >
>
> I agree. I even mentioned this in
> https://reviews.freebsd.org/D26041#589287.
> This change avoids the two unnecessary syscalls, but I agree that
> longer-term install should share the copy_file_range code with cp.
> The only thing that copy_file_range won't speed up is the check
> whether source and target are already identical.
>
> Alex
>
> > On 10/14/20, Alex Richardson  wrote:
> > > Author: arichardson
> > > Date: Wed Oct 14 12:28:41 2020
> > > New Revision: 366697
> > > URL: https://svnweb.freebsd.org/changeset/base/366697
> > >
> > > Log:
> > >   install(1): Avoid unncessary fstatfs() calls and use mmap() based on
> size
> > >
> > >   According to git blame the trymmap() function was added in 1996 to
> skip
> > >   mmap() calls for NFS file systems. However, nowadays mmap() should be
> > >   perfectly safe even on NFS. Importantly, onl ufs and cd9660 file
> systems
> > >   were whitelisted so we don't use mmap() on ZFS. It also prevents the
> use
> > >   of mmap() when bootstrapping from macOS/Linux since on those systems
> the
> > >   trymmap() function was always returning zero due to the missing
> > > MFSNAMELEN
> > >   define.
> > >
> > >   This change keeps the trymmap() function but changes it to check
> whether
> > >   using mmap() can reduce the number of system calls that are required.
> > >   Using mmap() only reduces the number of system calls if we need
> multiple
> > > read()
> > >   syscalls, i.e. if the file size is > MAXBSIZE. However, mmap() is
> more
> > > expensive
> > >   than read() so this sets the threshold at 4 fewer syscalls.
> Additionally,
> > > for
> > >   larger file size mmap() can significantly increase the number of page
> > > faults,
> > >   so avoid it in that case.
> > >
> > >   It's unclear whether using mmap() is ever faster than a read with an
> > > appropriate
> > >   buffer size, but this change at least removes two unnecessary system
> > > calls
> > >   for every file that is installed.
> > >
> > >   Reviewed By:markj
> > >   Differential Revision: https://reviews.freebsd.org/D26041
> > >
> > > Modified:
> > >   head/usr.bin/xinstall/xinstall.c
> > >
> > > Modified: head/usr.bin/xinstall/xinstall.c
> > >
> ==
> > > --- head/usr.bin/xinstall/xinstall.c  Wed Oct 14 10:12:39 2020
> (r366696)
> > > +++ head/usr.bin/xinstall/xinstall.c  Wed Oct 14 12:28:41 2020
> (r366697)
> > > @@ -148,7 +148,7 @@ static void   metadata_log(const char *, const
> char *, s
> > >   const char *, const char *, off_t);
> > >  static int   parseid(const char *, id_t *);
> > >  static int   strip(const char *, int, const char *, char **);
> > > -static int   trymmap(int);
> > > +static int   trymmap(size_t);
> > >  static void  usage(void);
> > >
> > >  int
> > > @@ -1087,7 +1087,7 @@ compare(int from_fd, const char *from_name
> __unused,
> > > s
> > >   if (do_digest)
> > >   digest_init();
> > >   done_compare = 0;
> > > - if (trymmap(from_fd) && trymmap(to_fd)) {
> > > + if (trymmap(from_len) && trymmap(to_len)) {
> > >   p = mmap(NULL, from_len, PROT_READ, MAP_SHARED,
> > >   from_fd, (off_t)0);
> > >   if (p == MAP_FAILED)
> > > @@ -1248,13 +1248,8 @@ copy(int from_fd, const char *from_name, int
> to_fd,
> > > co
> > >
> > >   digest_init();
> > >
> > > - /*
> > > -  * Mmap and write if less than 8M (the limit is so we don't
> totally
> > > -  * trash memory on big files.  This is really a minor hack, but
> it
> > > -  * wins some CPU back.
> > > -  */
> > >   done_copy = 0;
> > > - if (size <= 8 * 1048576 && trymmap(from_fd) &&
> > > + if (trymmap((size_t)size) &&
> > >   (p = mmap(NULL, (size_t)size, PROT_READ, MAP_SHARED,
> > >   from_fd, (off_t)0)) != MAP_FAILED) {
> > >   nw = write(to_fd, p, size);
> > > @@ -1523,20 +1518,23 @@ usage(void)
> > >   *   return true (1) if mmap should be tried, false (0) if not.
> > >   */
> > >  static int
> > > -trymmap(int fd)
> > > +trymmap(size_t filesize)
> > >  {
> > > -/*
> > > - * The ifdef is for bootstrapping - f_fstypename doesn't exist in
> > > - * pre-Lite2-merge systems.
> > > - */
> > > -#ifdef MFSNAMELEN
> > > - struct statfs stfs;
> > > -
> > > - if (fstatfs(fd, ) != 0)
> > > - return (0);
> > > - if (strcmp(stfs.f_fstypename, "ufs") == 0 ||
> > > - 

Re: svn commit: r368736 - head/tools/tools/git/hooks

[Ed Maste]

On Mon, 28 Dec 2020 at 14:34, Alan Somers  wrote:
>
> On Thu, Dec 17, 2020 at 12:58 PM Ed Maste  wrote:
>>
>> Author: emaste
>> Date: Thu Dec 17 19:58:29 2020
>> New Revision: 368736
>> URL: https://svnweb.freebsd.org/changeset/base/368736
>>
>> Log:
>>   Add initial version of git commit message preparation hook
>>
> Thanks!  But how can we use it?  Is there a "git config" incantation that 
> will use this file?

Just copy it into .git/hooks/. Can also fetch the latest version
directly from cgit; lwhsu added this to
https://github.com/bsdimp/freebsd-git-docs/blob/main/URLs.md:

fetch 
https://cgit.freebsd.org/src/plain/tools/tools/git/hooks/prepare-commit-msg
-o .git/hooks
chmod 755 .git/hooks/prepare-commit-msg

I'd like us to end up with a setup-repo.sh or such that will offer to
take care of all of these things.

>Also, this seems too complicated.  Git already has a "config.template" 
>configuration variable.   Could we just use that instead, or does it not work 
>for some reason?

It does work, although it's a bit awkward. For one thing, the provided
template is placed at the top of the message, followed by the default
message, but our addition is really commented-out information and fits
best in the middle of the existing text - after the "Please enter
the..." and before the list of changes and such. With config.template
we'd end up with something like:



# PR:   
# ...
# Differential Revision:
#
# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# ...


Additionally we might want to make some more programmatic changes in
the default message (depending on the resolution of some open
questions). For example we could strip the "MFC after" line that's
copied from the original commit, when preparing the commit message for
a MFC/cherry-pick.
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r368736 - head/tools/tools/git/hooks

[Alan Somers]

On Thu, Dec 17, 2020 at 12:58 PM Ed Maste  wrote:

> Author: emaste
> Date: Thu Dec 17 19:58:29 2020
> New Revision: 368736
> URL: https://svnweb.freebsd.org/changeset/base/368736
>
> Log:
>   Add initial version of git commit message preparation hook
>
>   Start with a slightly modified version of the SVN commit template, to
>   allow developers to experiment.  This will be updated in the future as
>   our process and techniques evolve.
>
>   This can be installed by copying or symlinking into the .git/hooks/
>   directory.
>
>   Feedback from:cem, jhb
>   Reviewed by:  imp
>   Sponsored by: The FreeBSD Foundation
>   Differential Revision:https://reviews.freebsd.org/D27633
>
> Added:
>   head/tools/tools/git/hooks/
>   head/tools/tools/git/hooks/prepare-commit-msg   (contents, props changed)
>

Thanks!  But how can we use it?  Is there a "git config" incantation that
will use this file?  Also, this seems too complicated.  Git already has a
"config.template" configuration variable.   Could we just use that instead,
or does it not work for some reason?
-Alan
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"