[Dextrose] [PATCH 5/5] Sweet build

[Aleksey Lim]

On Sat, Oct 22, 2011 at 11:15:35AM -0700, Bernie Innocenti wrote:
> On Sat, 2011-10-22 at 01:39 +0000, Aleksey Lim wrote:
> 
> > diff --git a/Makefile.am b/Makefile.am
> > index 9e252af..cf526f8 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -9,6 +9,11 @@ EXTRA_DIST =                    \
> >  	$(bin_SCRIPTS)		\
> >  	intltool-merge.in       \
> >  	intltool-update.in      \
> > -	intltool-extract.in
> > +	intltool-extract.in \
> > +	sweets.recipe
> 
> The backslash isn't aligned here. Also, to reduce the churn in diffs
> when we append a new line, I'd recommend this style which is also used
> by some GNU projects:
> 
>     
> 	intltool-extract.in \
> 	sweets.recipe \
>         #

yup, makes sense

> >  DISTCHECK_CONFIGURE_FLAGS = --disable-update-mimedb
> > +
> > +all:
> > +	mkdir -p share
> > +	test -e share/sugar || ln -s .. share/sugar
> 
> 1. Shouldn't these actions happen on make install?
> 
> 2. This will break when we're building outside of srcdir. You should use
> automake variables to determine what's the top of the source dir.
> 
> 3. I'd recommend letting make determine when to run these commands
> 
> Here's how I'd rewrite this:
> 
>   install: share/sugar
> 
>   share/sugar:
>           mkdir -p share
>           ln -sf $(top_srcdir) share/sugar
> 
> (top_srcdir is off my flaky memory, probably incorrect)

The purpose was different, to run sugar exactly from the sources dir
(not from installed dir, ie, you change the code and run w/o `make install`).
Though, this code might be moved to sweets.recipe's install option.

> > +sugar:
> > +	ln -fs sugar.in sugar
> 
> If sugar is no longer being generated from sugar.in, then we could just
> rename the source file to sugar instead of symlinking it!

That was done to minimize sweet patch, ie, was fine for keeping sweet
patch all time on top of the trunk. Renaming might be done in trunk.

> >  if test -z "$SUGAR_SCALING"; then
> > -    export SUGAR_SCALING=100
> > +    dpi=$(xdpyinfo | grep resolution: | grep -o '[0-9]\+' | head -n1)
> > +    if [ $dpi -ge 200 ]; then
> > +        export SUGAR_SCALING=100
> > +    else
> > +        export SUGAR_SCALING=72
> > +    fi
> >  fi
> 
> Finally! However, this is unrelated to sweets and should come as a
> separate patch.

I remember bunch of patches/tries to set right SUGAR_SCALING,
just wasn't going to take part in one more endless discussion.

> Perhaps it could also be simplified using
> 
>   xprop -root RESOURCE_MANAGER

e.g.

    dpi=$(xprop -root RESOURCE_MANAGER | sed 's/.*Xft.dpi:[^0-9]*\([0-9]\+\).*/\1/')

:)

> > -exec sugar-session
> > +# Launch new DBus session to take into account new DBus services came from sweets
> > +exec dbus-launch --exit-with-session sugar-session "$@"
> 
> Didn't we get rid of the sugar-specific dubs session a while ago? Why do
> we need to reintroduce it now?

For DBus services installed to /usr, not starting new session is fine,
but here, sugar services will come from sweets, ie, via XDG_DATA_DIRS
envar that can be taken into account only after starting new session.

--

Eventually, I think it will be useless to merge sweets patches w/
dextrose's ones, seet pactches are needed only for Sweets and live
in sdk/* repos that will exist anyway.

-- 
Aleksey