Hi, The script worked great here. My only suggestion would be to use Python instead, so that you could use a function to generate a "safe" temporary filename instead of /tmp/.localsync (not that I think it would ever matter in practice). All in all, +1 :) -- Cordially,  Alexander Rødseth  Arch Linux Trusted User  (xyproto on IRC, trontonic on AUR)

Python, yes, very seriously. Why not? It is installed practically everywhere and python scripts are comparable to shell scripts in speed and size. Not to mention not having to execute external tools, a lot nicer syntax, the possibility to compile some classes of scripts to native (with shedskin). Etc, etc. Ofc, mktemp -d also works. - Alexander

On 10/22/11, Pierre Schmitz <pierre@archlinux.de> wrote:

Why not just use "mktemp -d"? Should be save enough.

Because that makes the script 10x more complicated or 10x slower. Either you have to store the tempdir path somewhere (/run/checkupdates.tempdir) or you delete the fakedb and redownload all the sync repos each call. The only benefit I can see would be slightly better support on multiuser systems, but you'll be running into issues there from pacman's own lockfile. If multiuser support is an issue, the best way around that would be fakedb="${fakedb:-/tmp/.localsync/}${USER}/" Allowing two people to run checkupdates at the same time. -Kyle http://kmkeen.com

On Tue, Oct 25, 2011 at 12:34 AM, keenerd <keenerd@gmail.com> wrote:

Here is a version that conforms to more of our coding guidelines.

Any feedback?

-Kyle http://kmkeen.com

Seems to work :-) This is what I got when I run it the first time: [karol@black test]$ ./checkupdates warning: database file for 'testing' does not exist warning: database file for 'community-testing' does not exist warning: database file for 'core' does not exist warning: database file for 'extra' does not exist warning: database file for 'community' does not exist warning: database file for 'xyne-any' does not exist warning: database file for 'archlinuxfr' does not exist warning: database file for 'archstuff' does not exist warning: database file for 'herecura-stable-any' does not exist warning: database file for 'herecura-stable' does not exist warning: database file for 'andrwe' does not exist warning: database file for 'rfad' does not exist warning: database file for 'heftig' does not exist freetype2 mkinitcpio mkinitcpio-busybox xorg-server xorg-server-common xorg-sessreg xorg-xdpyinfo The second time it was just [karol@black test]$ ./checkupdates freetype2 mkinitcpio mkinitcpio-busybox xorg-server xorg-server-common xorg-sessreg xorg-xdpyinfo Can you rearrange your script so that the warnings go away?

On 10/24/11, Karol Blazewicz <karol.blazewicz@gmail.com> wrote:

Can you rearrange your script so that the warnings go away?

Whoops. I added that bug in while trying to avoid routing everything to /dev/null so legit errors could be seen. Guess that won't be an option. Fixed. -Kyle http://kmkeen.com

On 10/24/11, Karol Blazewicz <karol.blazewicz@gmail.com> wrote:

Are you sure about

pacman -Qqu --dbpath "$fakedb" &> /dev/null

?

Well that is embarrassing. Long day. -Kyle http://kmkeen.com

On Wed, Oct 26, 2011 at 01:26:57PM +1000, Allan McRae wrote:

A few minor comments:

1) I think you need to clean up the usage message a bit - maybe like this:

checkupdate: Safely print a list of pending updates Usage: checkupdates

Export the "fakedb" variable to change to path to the fake database.

I still do not like the term "fake database" as it is not clear what that is....

Fake in the sense that pacman is reading from a different sync database than it normally would. I agree that fake probably isn't the best way to describe it, but it's what I used to describe it with when I originally wrote this.

2) Change fakedb to FAKEDB? Capitals seem the way to go. Should we also get a PAC/PACMAN in there? FAKEPACDB?

As long as we're bikeshedding, I vote to avoid the capitalized variable names wherever possible, as they should be reserved for system variables set by external processes or your shell.

3) I'd also change .localsync to something more descriptive. .pacmansync? .pacdb?

Agree, but I'll also add: please do not hide this with a leading dot. There's no reason to.

Anyway, as I said, mostly bikeshed worthy stuff here. Create a git format patch, remembering to add the entries to Makefile.am and .gitignore in the contrib/ directory

Allan

On 10/25/11, Allan McRae <allan@archlinux.org> wrote:

1) I think you need to clean up the usage message a bit - maybe like this:

Definitely.

2) Change fakedb to FAKEDB? Capitals seem the way to go. Should we also get a PAC/PACMAN in there? FAKEPACDB?

One vote for caps.

3) I'd also change .localsync to something more descriptive. .pacmansync? .pacdb?

One vote for 'pac' in the db path. On 10/26/11, Dave Reisner <d@falconindy.com> wrote:

Fake in the sense that pacman is reading from a different sync database than it normally would. I agree that fake probably isn't the best way to describe it, but it's what I used to describe it with when I originally wrote this.

One vote for status quo ;-)

As long as we're bikeshedding, I vote to avoid the capitalized variable names wherever possible, as they should be reserved for system variables set by external processes or your shell.

One vote for lowercase.

I'll also add: please do not hide this with a leading dot. There's no reason to.

One vote for no dot. On 10/26/11, Dan McGee <dpmcgee@gmail.com> wrote:

Why not just use the program name prefix as is the usual convention and go with CHECKUPDATES_DATABASE?

And this is totally broken- you can't try to segregate by $USER in a subdirectory as the first person to create .localsync wins. Just create a single top-level directory, 'checkupdates-${USER}/' or something.

Two votes for no dot. The only reason for dotting the db directory was to make it less likely to be deleted. Save hitting your mirror as much. Kind of silly. Why make a (hypothetical) bunch of directories when there could be just one with liberal write permissions? Though 0777 perms always bug me, so I guess not. Single user would be the common case so it is not like it matters too much. Going with you here.

Finally, noticing the `rm -f db.lck` bit- please tell me that isn't necessary?

Probably necessary. It is extremely easy to interrupt pacman and leave a lock file floating around. Since this is basically an automatic read-only db there is no harm in keeping it that way and making sure it will work consistently. Keeping this for now unless someone has a very compelling reason otherwise. Three different opinions on env vars, why do you all have to be devs I like. How is this one? Main changes are description, CHECKUPDATE_DB and /tmp/checkup-db-${USER}/ -Kyle http://kmkeen.com

On Wed, Oct 26, 2011 at 08:40:37AM -0400, Dave Reisner wrote:

Currently, lock files are per pacman install, not per DB.

ignore this. shows what i know...