Feh / nocache

minimize caching effects
BSD 2-Clause "Simplified" License
554 stars 53 forks source link

initial debianization + some minor changes #8

Closed noushi closed 11 years ago

noushi commented 11 years ago

Hi Julius,

so, as we discussed, here is: . the initial debianization . the inclusion of cachestats and cachedel in the installed programs . a manpage for all 3 commands

cheers, Reda

onlyjob commented 11 years ago

Hold on, the debianisation is already done officially. :)

noushi commented 11 years ago

Oh, my bad, I only searched the package in ubuntu. Nevermind it, then. :)

Feh commented 11 years ago

First of all: Sorry, @Reda, that you undertook all the work only to find someone else did it. Until ten minutes ago I didn’t know myself that this tool was to come to Debian unstable.

@Dmitry: Thanks for packaging it! This is really nice. I have a couple of questions regarding this:

I’m open to your suggestions. Thanks!

onlyjob commented 11 years ago

@Feh: Thank you for your fantastic utility. :)

There are different packaging work flows. Although for nocache it wouldn't hurt to have upstream branch in many cases I believe it is unnecessary to mix packaging and development. Since you're not tagging your releases yet the only way to produce orig.tar is to use get-orig-source target of rules file. If/when tags would be available in nocache repository the classic invocation of uscan will also work to get orig.tar.

Regarding #5 it is a little divergence that I had to introduce to effectively deal with multiarch path to library. Later I'll have a look again to check if it is better to use (patched?) original nocache executable rather than one from debian/ directory.

IMHO Makefile is not usable as is (no $DESTDIR etc.) and I see little value in building with "-DDOUBLEFADVISE". I'm very happy with test results of standard build and I'm not sure if alternative version would make sense especially as workaround for a bug(?) that is due to be fixed? I didn't research that matter deeply and I'm open to discussion.

It is up to you to introduce version number. Could be a good idea. :) Tagging releases would be very useful to mark milestones. I like to embed date to version number to provide meaningful information for users about when software was updated but that's just me.

Thank you for your feedback. Let's stay in touch -- I'm always reachable by email.

Regards, Dmitry.

noushi commented 11 years ago

@Julius, no worries, it's actually good to have it already maintained.

@onlyjob , are there any plans for making nocache available on ubuntu (raring or ulterior)?

regarding the double fadvise, using it by default would be okay when it works (i.e. return value == 0), but on error, I think that either one of these behaviors should be implemented: . an early exit (without the second fadvise) . an aggregation of fadvise return values should be returned

@onlyjob , would these changes to Makefile's install target make it more compliant? (that's what I changed) install: all install -m 0644 nocache.so $(DESTDIR)/usr/lib install -m 0755 nocache.global $(DESTDIR)/usr/bin/nocache install -m 0755 cachedel $(DESTDIR)/usr/bin/cachedel install -m 0755 cachestats $(DESTDIR)/usr/bin/cachestats

cheers, Reda

dmitry commented 11 years ago

Sorry for interrupting you guys, but I wish you would use @onlyjob instead of @dmitry, or just Dmitry (without @). Thanks.

noushi commented 11 years ago

@onlyjob sure :)

@Feh, the lwn article dates back to 2011, and indeed NOREUSE is a nop in mm/fadvise.c up to the current unstable version ( https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/mm/fadvise.c?id=refs/tags/v3.9-rc8 ).

regarding nocache script, you could use a variable like BASEDIR=/usr (or /usr/local ) and make install copies nocache in DESTDIR runs a sed "s|^BASEDIR=.*|BASEDIR=$(DESTDIR)/usr/lib".

this will allow nocache to:

  1. compile and work in the local directory
  2. install in the default directory (/usr/local but I'd suggest you install to /usr)
  3. compile and work in /usr/ while complying with DESTDIR requirement for debian packaging

cheers, Reda

onlyjob commented 11 years ago

@dmitry, thanks for your correction. I updated my comment as you advised.

@noushi, regarding $(DESTDIR) that's how it should be done according to GNU Make best practice. However I didn't test updated Makefile so I can't comment (yet) if more changes are needed. This is not a Debian requirement but just a good practice for makefiles. Thanks.

I don't care about Ubuntu (I wish everyone would just use Debian) and I don't know if there are plans to introduce nocache to Ubuntu. Eventually they'll probably take nocache from Debian just like they take other packages. I don't know how this process initiated for new packages.

Feh commented 11 years ago

@onlyjob, about the “do the posix_fadvise twice” thing: I have at least one machine where make test fails if I don’t do at least two passes. (This might be a timing issue when you have several CPU cores; it’s not technically a bug, and it won’t be fixed. The LWN article describes the patches, but they were not accepted because they changed the semantics of flags like POSIX_FADV_DONTNEED).

I have implemented a command line option now (see branch n-times-fadvise). But I didn’t touch the man pages yet, because right now there are two versions: one here, one in your Debian repo – they would be out of sync. Can you just make a symlink to the man/ directory in the packaging repo, maybe?

The same problem arises with the nocache shell script. I think it would be good to have it in only one place. Can we find a solution like @noushi indicated with sed? That would be nice.

onlyjob commented 11 years ago

Thanks for your explanation. I've noticed that one of the tests fails most of the time, that's why result of post-build tests is ignored during package build.

Implementing "-DDOUBLEFADVISE" as an option is the best thing to do. This awesome change is certainly worth to be in master branch (I didn't inspect your changes).

Please don't you worry about our divergence. I wrote man pages because there were none. Than I forwarded them to you and you accepted. With next release I can drop man pages from packaging. Please don't hesitate to update man pages just because we still have original ones in Debian.

I will have a look at the shell script next time when I will be updating nocache in Debian. Of course we can patch or modify it with regex.

Thank you.

Feh commented 11 years ago

Thanks for all the suggestions! I merged the n-times-fadvise branch and another change from @noushi. Also, I pushed a tag with version number 0.8.