On Wed, Jan 31, 2018 at 1:44 PM, Jeroen Demeyer <j.deme...@ugent.be> wrote:
> On 2018-01-31 12:21, Erik Bray wrote:
>>
>> I'm not going to say I didn't expect pushback from you on this, but I
>> feel like this is not a real argument.
>
>
> I assure you that it was meant as a serious argument. I'm not complaining
> for the sake of complaining.
Sorry, I know you mean well and wouldn't "complain for the sake of
complaining". But it doesn't come off as a serious argument.
>> In fact, the old code was still essentially doing the same
>> thing, but in the form of some ugly shell code buried in a
>> configure.ac and not even in the makefile itself.
>
>
> That's not "essentially the same thing" at all.
Hmm--I think I was imprecise here so maybe we're not talking about the
same thing. All I meant was that the resulting make rules are exactly
the same. All that's changed is how they're presented. I'm replacing
this:
DEP_FILE="$SAGE_ROOT/build/pkgs/$PKG_NAME/dependencies"
TYPE=`cat "$SAGE_ROOT/build/pkgs/$PKG_NAME/type"`
if [ -f "$DEP_FILE" ]; then
# DEPS is first line of $DEP_FILE with some modifications:
# - start with a single space
# - the # symbol is treated as comment which is removed
# - lower-case words "foo" are replaced by "$(inst_foo)"
DEPS=`sed 's/^ */ /; s/ *#.*//; s/ \([a-z0-9][a-z0-9_]*\)/
$(inst_\1)/g; q' $DEP_FILE`
<...snip dozens of lines of unrelated stuff...>
else
# Normal Sage packages
echo >&7 "\$(INST)/$PKG_VERSION:$DEPS"
echo >&7 " +\$(A""M_V_at)sage-logger -p '\$(SAGE_SPKG)
$PKG_VERSION' '\$(SAGE_LOGS)/$PKG_VERSION.log'"
echo >&7
# Add a target with just the bare package name for "sage -i"
echo >&7 "$PKG_NAME: \$(INST)/$PKG_VERSION"
echo >&7
# Add a target to remove the "installed" file for "sage -f"
echo >&7 "$PKG_NAME-clean:"
echo >&7 " rm -f \$(INST)/$PKG_VERSION"
echo >&7
with this:
define NORMAL_PACKAGE_templ
$$(INST)/$(1)-$$(vers_$(1)): $$(foreach dep,$$(deps_$(1)),$$(inst_$$(dep)))
+$(AM_V_at)sage-logger -p '$(SAGE_SPKG) $(1)-$$(vers_$(1))'
'$(SAGE_LOGS)/$(1)-$$(vers_$(1)).log'
$(1): $$(INST)/$(1)-$$(vers_$(1))
$(1)-clean:
rm -rf $$(INST)/$(1)-$$(vers_$(1))
endef
$(foreach pkgname,$(NORMAL_PACKAGES),\
$(eval $(call NORMAL_PACKAGE_templ,$(pkgname))))
I will gladly admit that the latter isn't very pretty. The worst
aspect is the need to escape dollar signs some times as $$ (why--*why*
does make use $ when it also tends to involve shell code which also
uses $--surely they could have used a different character).
That said, the former shell code also needs to escape some dollar
signs, albeit with \$. Just as much potential there for mistakes and
unclarity.
>> An apt analogy
>> would be a web server generating HTML by printf statements versus
>> using a template format intended for HTML.
>
>
> That web server would still serve plain HTML in the end, so the analogy
> doesn't work. I have nothing against templates or auto-generating stuff.
> What I dislike with the Makefile is that the templates are in the end
> product instead of in an intermediate step.
That's fair. I understand--you don't like that you don't immediately
see the make rules expanded. Yet on the same token even with the old
Makefile you still don't see the *fully* expanded rules directly in
the file. There are still variables that are left unexpanded. What
you see written in a Makefile is rarely ever exactly the final product
of what make uses when it comes to actually parsing out rules. What
you're complaining about amounts to leaving C macros unexpanded--you
want to see the "real" C code with no macros. The only difference
with a template in make is that it allows loops, while C macros do not
(a problem that Numpy, for example, solves with its own custom
template format).
Anyways, you can *always* see the fully expanded rules by running
`make -p` though this tends to be very wordy and it can be hard to
find exactly what you're looking for (but the same can often be said
about, say, `gcc -E`).
I also demonstrated in my last e-mail how I could easily augment this
to make debugging easy.
>> Now, I thought that the comments I included in the file made it
>> reasonably clear what was going on
>
>
> I didn't even look at the file. I'm just complaining against the principle.
Now wait, wait, wait. Serious argument or not, that's hardly fair.
What principle, exactly, are you complaining against? Jeroen, you're
very smart, and very careful and precise, so I value your opinion and
feedback in general. But please, don't waste my time with this
argument when you haven't even done your homework.
>> Your only point about debugging is extremely hypothetical--exactly
>> what kind of "obscure bug in the build system" do you anticipate being
>> obscured by this?
>
>
> Typically, some dependency tracking bug. As in "why is it rebuilding this"
> or "why is it not building this"?
I don't understand. The package dependencies are already directly in
the metadata. There's literally no more information you would gain by
seeing these loops unrolled.
>> In summary, I think it's strange to complain about this with autoconf
>> itself is little more than a massive pile of text macros and is *much*
>> harder to understand and to debug than this.
>
>
> Again: with "autoconf" you *can* look at the generated "./configure" file.
> And I actually do that sometimes and I also edit that file sometimes to
> debug stuff.
Let me step back a little bit. Forget about the makefile itself for a sec.
Part of the point of this is that we aren't really using autoconf as
it's intended in the first place. The configure script itself is not
really supposed to write files at all, except for a few files specific
to the autoconf system. The purpose is the configure script is to
*collect information* about the system and the build context. That's
it. It's for information gathering. And the way I've tweaked it is
exactly that--it scans the SPKGs and gathers information about them
into some datastructures that can be used by make, from which I tried
to reduce as much redundancy as possible.
When the configure script completes its information gathering it
writes one file, config.status, into which it also writes whatever
information gathered by configure we most care about (as specified by
AC_SUBST(...)). The AC_SUBST variables are then replaced into
templates by config.status--it's the script that's actually supposed
to be writing files.
Now, of course, you'll object--configure is just a shell script, and
we can do absolutely anything we want in it. Sure. I admit I don't
have a great technical argument against this. I think that it's a bit
prettier. The AC_SUBST variables I gather, I think, do a fair bit to
eliminate redundancy, and the use of (autoconf) templates is a bit
prettier. I think my configure.ac is a lot simpler and easier to
understand (and even moreso in one of my other branches of this where
I've moved all the package scraping into a AC_DEFUN macro in a
separate file).
I'll also mention, again, that this has made running ./configure
*much* faster, and the impact it has on running make is minimal.
At the end of the day, I just feel that if we're going to use these
tools at all it actually makes sense to use them closer to how they
were intended.
Best,
E
--
You received this message because you are subscribed to the Google Groups
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to sage-devel+unsubscr...@googlegroups.com.
To post to this group, send email to sage-devel@googlegroups.com.
Visit this group at https://groups.google.com/group/sage-devel.
For more options, visit https://groups.google.com/d/optout.
