[PATCH v2] hooks/pre-auto-gc-battery: allow gc to run on non-laptops

2018-02-28 Thread Adam Borowski
Desktops and servers tend to have no power sensor, thus on_ac_power returns
255 ("unknown").  Thus, let's take any answer other than 1 ("battery") as
no contraindication to run gc.

If that tool returns "unknown", there's no point in querying other sources
as it already queried them, and is smarter than us (can handle multiple
adapters).

Reported by: Xin Li <delp...@google.com>
Signed-off-by: Adam Borowski <kilob...@angband.pl>
---
v2: improved commit message

 contrib/hooks/pre-auto-gc-battery | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/hooks/pre-auto-gc-battery 
b/contrib/hooks/pre-auto-gc-battery
index 6a2cdebdb..7ba78c4df 100755
--- a/contrib/hooks/pre-auto-gc-battery
+++ b/contrib/hooks/pre-auto-gc-battery
@@ -17,7 +17,7 @@
 # ln -sf /usr/share/git-core/contrib/hooks/pre-auto-gc-battery \
 #  hooks/pre-auto-gc
 
-if test -x /sbin/on_ac_power && /sbin/on_ac_power
+if test -x /sbin/on_ac_power && (/sbin/on_ac_power;test $? -ne 1)
 then
exit 0
 elif test "$(cat /sys/class/power_supply/AC/online 2>/dev/null)" = 1
-- 
2.16.2


Re: [PATCH] hooks/pre-auto-gc-battery: allow gc to run on non-laptops

2018-02-28 Thread Adam Borowski
On Wed, Feb 28, 2018 at 10:16:21AM -0800, Junio C Hamano wrote:
> Adam Borowski <kilob...@angband.pl> writes:
> 
> > Desktops and servers tend to have no power sensor, thus on_ac_power returns
> > 255 ("unknown").
> >
> > If that tool returns "unknown", there's no point in querying other sources
> > as it already queried them, and is smarter than us (can handle multiple
> > adapters).
> 
> The explanation talks about the exit status 255 being special and
> serves to signal "there is no point continuing, and it is OK to
> assume we are not on batttery", while the code says that anything
> but exit status 1 can be treated as such.  Which is correct?

As the man page says:

# EXIT STATUS
#   0 (true)  System is on mains power
#   1 (false) System is not on mains power
#   255 (false)Power status could not be determined

0 usually means a laptop on AC power, 255 is for a typical desktop.
The current code can't return 2 or any other unexpected value, but if it
ever does, an unknown error should probably be treated same as 255 unknown.
Thus, gc should be avoided only if the return code is 1.

As for the second paragraph, I meant that on_ac_power already queried all
sources this hook knows about (other than /usr/bin/pmset which is OSX
only[1]), thus if the answer is "unknown", continuing to query is redundant.

If that's unclear, do you have some other wording in mind?

Also, it's good to trust on_ac_power, as it'll get updated whenever new
quirks of power management get known: I heard allegations that some boards
say "USB" instead of "Mains", which should count the same for our
purposes[2].  It's not reasonable to update consumers such as git instead of
a single system-provided tool.

One worry is that, if on_ac_power is not installed, other sources known by
this hook likewise assume that unknown means battery.  And for example on
Debian, powermgmt-base (which is where on_ac_power lives) is no longer
installed by default.  This suggests this patch needs to be extended to
cover the other sources as well, but let's discuss this first.  Extra
commits are cheap...

> > Reported by: Xin Li <delp...@google.com>
> > Signed-off-by: Adam Borowski <kilob...@angband.pl>
> > ---
> >  contrib/hooks/pre-auto-gc-battery | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/contrib/hooks/pre-auto-gc-battery 
> > b/contrib/hooks/pre-auto-gc-battery
> > index 6a2cdebdb..7ba78c4df 100755
> > --- a/contrib/hooks/pre-auto-gc-battery
> > +++ b/contrib/hooks/pre-auto-gc-battery
> > @@ -17,7 +17,7 @@
> >  # ln -sf /usr/share/git-core/contrib/hooks/pre-auto-gc-battery \
> >  #  hooks/pre-auto-gc
> >  
> > -if test -x /sbin/on_ac_power && /sbin/on_ac_power
> > +if test -x /sbin/on_ac_power && (/sbin/on_ac_power;test $? -ne 1)
> >  then
> > exit 0
> >  elif test "$(cat /sys/class/power_supply/AC/online 2>/dev/null)" = 1
> 


[1]. I don't know if there's an implementation of on_ac_power for OSX, but
if there is, it is reasonable to assume it uses or emulates pmset.

[2]. Technically, that's _dc_ not ac power, but as batteries use a different
interface, in the vast majority of cases USB power can be considered
non-rationed.  You can power it from an unplugged laptop or from a
powerbank, but that's no different from "mains" that come from an unplugged
UPS with no or unsupported control link.
-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ A dumb species has no way to open a tuna can.
⢿⡄⠘⠷⠚⠋⠀ A smart species invents a can opener.
⠈⠳⣄ A master species delegates.


[PATCH] hooks/pre-auto-gc-battery: allow gc to run on non-laptops

2018-02-27 Thread Adam Borowski
Desktops and servers tend to have no power sensor, thus on_ac_power returns
255 ("unknown").

If that tool returns "unknown", there's no point in querying other sources
as it already queried them, and is smarter than us (can handle multiple
adapters).

Reported by: Xin Li <delp...@google.com>
Signed-off-by: Adam Borowski <kilob...@angband.pl>
---
 contrib/hooks/pre-auto-gc-battery | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/hooks/pre-auto-gc-battery 
b/contrib/hooks/pre-auto-gc-battery
index 6a2cdebdb..7ba78c4df 100755
--- a/contrib/hooks/pre-auto-gc-battery
+++ b/contrib/hooks/pre-auto-gc-battery
@@ -17,7 +17,7 @@
 # ln -sf /usr/share/git-core/contrib/hooks/pre-auto-gc-battery \
 #  hooks/pre-auto-gc
 
-if test -x /sbin/on_ac_power && /sbin/on_ac_power
+if test -x /sbin/on_ac_power && (/sbin/on_ac_power;test $? -ne 1)
 then
exit 0
 elif test "$(cat /sys/class/power_supply/AC/online 2>/dev/null)" = 1
-- 
2.16.2



[BUG] auto-repack exits prematurely, locking other processing out

2014-05-23 Thread Adam Borowski
Hi guys!

It looks like the periodic auto-repack backgrounds itself when it shouldn't
do so.  This causes the command it has triggered as a part of to fail:

==
[~/linux](master)$ git pull --rebase
remote: Counting objects: 455, done.
remote: Compressing objects: 100% (64/64), done.
remote: Total 267 (delta 208), reused 262 (delta 203)
Receiving objects: 100% (267/267), 44.43 KiB | 0 bytes/s, done.
Resolving deltas: 100% (208/208), completed with 80 local objects.
From git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux
   4b660a7..f02f79d  master - linus/master
Auto packing the repository in background for optimum performance.
See git help gc for manual housekeeping.
First, rewinding head to replay your work on top of it...
Applying: perf: tools: fix missing casts for printf arguments.
Applying: vt: emulate 8- and 24-bit colour codes.
fatal: Unable to create '/home/kilobyte/linux/.git/refs/heads/master.lock': 
File exists.

If no other git process is currently running, this probably means a
git process crashed in this repository earlier. Make sure no other git
process is running and remove the file manually to continue.
Could not move back to refs/heads/master
[~/linux]((no branch, rebasing (null)))$
==

-- 
Gnome 3, Windows 8, Slashdot Beta, now Firefox Ribbon^WAustralis.  WTF is going
on with replacing usable interfaces with tabletized ones?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG] auto-repack exits prematurely, locking other processing out

2014-05-23 Thread Adam Borowski
On Fri, May 23, 2014 at 02:40:41PM -0700, Junio C Hamano wrote:
 Adam Borowski kilob...@angband.pl writes:
  It looks like the periodic auto-repack backgrounds itself when it shouldn't
  do so.  This causes the command it has triggered as a part of to fail:
 
 Duy, 9f673f94 (gc: config option for running --auto in background,
 2014-02-08) turns to be not such a hot idea.  Sure, if we kick it
 off background after doing something heavy, immediately before
 giving control back to the end-user, and expect that the user will
 stay thinking without making new changes (i.e. read-only stuff like
 git show would be OK), then daemonize might be a great thing, but
 we forgot, while doing that commit, that long-running operations
 trigger the auto gc in the middle *and* they want it finish before
 they continue, as the purpose of gc is to help the performance
 during their further operation.

Just add a lock that's triggered by daemonize, and have things block on this
lock.  This handles all cases:
* --auto in the middle of a command: the block will kick in immediately,
  effectively reverting to non-daemonized version
* --auto at the end, the user does nothing: gc will finish its work in the
  background, just as you wanted
* --auto at the end, the user starts a new write two seconds later: gc works
  in the foreground with those 2 seconds headstart

The only loss is the lack of a progress indicator, and even that can be
done.

-- 
Gnome 3, Windows 8, Slashdot Beta, now Firefox Ribbon^WAustralis.  WTF is going
on with replacing usable interfaces with tabletized ones?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html