You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2009/05/02 00:22:26 UTC

Re: svn commit: r37531 - in trunk: . build

On Sat, May 2, 2009 at 01:46, Arfrever Frehtes Taifersar Arahesis
<Ar...@gmail.com> wrote:
>...
> +++ trunk/build/run_ctypesgen.sh        Fri May  1 16:46:38 2009        (r37531)
> @@ -13,15 +13,18 @@ CTYPESGEN="$5"
>  abs_srcdir="$6"
>  abs_builddir="$7"
>
> -svn_prefix="$8"
> +svn_libdir="$8"
>  apr_prefix="$9"
>  apu_prefix="${10}"
>
>  cp_relpath="subversion/bindings/ctypes-python"
> -### tempfile instead?
> -output="$abs_builddir/$cp_relpath/svn_all.py"
> +output="$cp_relpath/svn_all.py"

Why this change? The abs_builddir works just fine, so what are you
trying to accomplish here?

> -svn_includes="$abs_srcdir/subversion/include"
> +if test "$abs_builddir" = "$abs_srcdir"; then
> +  svn_includes="subversion/include"
> +else
> +  svn_includes="$abs_srcdir/subversion/include"
> +fi

Why this change? The abs_srcdir works, so there is never a need to
omit it. This "if" statement is just extra complexity.

>...
>  # This order is important. The resulting stubs will load libraries in
> @@ -54,6 +57,8 @@ if test "$apr_include_dir" != "$apu_incl
>   includes="$includes $apu_include_dir/ap[ru]_*.h"
>  fi
>
> +CPPFLAGS="`echo $CPPFLAGS`"
> +cppflags="`echo $cppflags`"

Why worry about extra spaces? What problem was it causing? This code
is spurious, unless there was a problem caused by those spaces.

>...

Cheers,
-g

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2022194


Re: svn commit: r37531 - in trunk: . build

Posted by Arfrever Frehtes Taifersar Arahesis <Ar...@GMail.Com>.
2009-05-02 03:53:56 Greg Stein napisał(a):
> Gotcha. Then *document* that.
> 
> Add some comments into the code for why the extra complexity is
> needed. It was *totally* unclear that those paths ended up in
> functions.py, and that your changes were to eliminate the absolute
> path prefix in the generated file.

I added some comments in r37537.

> On Sat, May 2, 2009 at 03:51, Arfrever Frehtes Taifersar Arahesis
> <ar...@gmail.com> wrote:
> > 2009-05-02 02:22:26 Greg Stein napisał(a):
> >> On Sat, May 2, 2009 at 01:46, Arfrever Frehtes Taifersar Arahesis
> >> <Ar...@gmail.com> wrote:
> >> >...
> >> > +++ trunk/build/run_ctypesgen.sh        Fri May  1 16:46:38 2009        (r37531)
> >> > @@ -13,15 +13,18 @@ CTYPESGEN="$5"
> >> >  abs_srcdir="$6"
> >> >  abs_builddir="$7"
> >> >
> >> > -svn_prefix="$8"
> >> > +svn_libdir="$8"
> >> >  apr_prefix="$9"
> >> >  apu_prefix="${10}"
> >> >
> >> >  cp_relpath="subversion/bindings/ctypes-python"
> >> > -### tempfile instead?
> >> > -output="$abs_builddir/$cp_relpath/svn_all.py"
> >> > +output="$cp_relpath/svn_all.py"
> >>
> >> Why this change? The abs_builddir works just fine, so what are you
> >> trying to accomplish here?
> >
> > I don't want to see build path in generated csvn/core/functions.py.
> > subversion/bindings/ctypes-python/setup.py doesn't add build path to
> > csvn/core/functions.py.
> >
> >> > -svn_includes="$abs_srcdir/subversion/include"
> >> > +if test "$abs_builddir" = "$abs_srcdir"; then
> >> > +  svn_includes="subversion/include"
> >> > +else
> >> > +  svn_includes="$abs_srcdir/subversion/include"
> >> > +fi
> >>
> >> Why this change? The abs_srcdir works, so there is never a need to
> >> omit it. This "if" statement is just extra complexity.
> >
> > I don't want to see build path in generated csvn/core/functions.py.
> > subversion/bindings/ctypes-python/setup.py doesn't add build path to
> > csvn/core/functions.py.
> >
> >> >...
> >> >  # This order is important. The resulting stubs will load libraries in
> >> > @@ -54,6 +57,8 @@ if test "$apr_include_dir" != "$apu_incl
> >> >   includes="$includes $apu_include_dir/ap[ru]_*.h"
> >> >  fi
> >> >
> >> > +CPPFLAGS="`echo $CPPFLAGS`"
> >> > +cppflags="`echo $cppflags`"
> >>
> >> Why worry about extra spaces? What problem was it causing? This code
> >> is spurious, unless there was a problem caused by those spaces.
> >
> > I want to avoid some spaces in generated csvn/core/functions.py.

-- 
Arfrever Frehtes Taifersar Arahesis

Re: svn commit: r37531 - in trunk: . build

Posted by Greg Stein <gs...@gmail.com>.
Gotcha. Then *document* that.

Add some comments into the code for why the extra complexity is
needed. It was *totally* unclear that those paths ended up in
functions.py, and that your changes were to eliminate the absolute
path prefix in the generated file.

-g

On Sat, May 2, 2009 at 03:51, Arfrever Frehtes Taifersar Arahesis
<ar...@gmail.com> wrote:
> 2009-05-02 02:22:26 Greg Stein napisał(a):
>> On Sat, May 2, 2009 at 01:46, Arfrever Frehtes Taifersar Arahesis
>> <Ar...@gmail.com> wrote:
>> >...
>> > +++ trunk/build/run_ctypesgen.sh        Fri May  1 16:46:38 2009        (r37531)
>> > @@ -13,15 +13,18 @@ CTYPESGEN="$5"
>> >  abs_srcdir="$6"
>> >  abs_builddir="$7"
>> >
>> > -svn_prefix="$8"
>> > +svn_libdir="$8"
>> >  apr_prefix="$9"
>> >  apu_prefix="${10}"
>> >
>> >  cp_relpath="subversion/bindings/ctypes-python"
>> > -### tempfile instead?
>> > -output="$abs_builddir/$cp_relpath/svn_all.py"
>> > +output="$cp_relpath/svn_all.py"
>>
>> Why this change? The abs_builddir works just fine, so what are you
>> trying to accomplish here?
>
> I don't want to see build path in generated csvn/core/functions.py.
> subversion/bindings/ctypes-python/setup.py doesn't add build path to
> csvn/core/functions.py.
>
>> > -svn_includes="$abs_srcdir/subversion/include"
>> > +if test "$abs_builddir" = "$abs_srcdir"; then
>> > +  svn_includes="subversion/include"
>> > +else
>> > +  svn_includes="$abs_srcdir/subversion/include"
>> > +fi
>>
>> Why this change? The abs_srcdir works, so there is never a need to
>> omit it. This "if" statement is just extra complexity.
>
> I don't want to see build path in generated csvn/core/functions.py.
> subversion/bindings/ctypes-python/setup.py doesn't add build path to
> csvn/core/functions.py.
>
>> >...
>> >  # This order is important. The resulting stubs will load libraries in
>> > @@ -54,6 +57,8 @@ if test "$apr_include_dir" != "$apu_incl
>> >   includes="$includes $apu_include_dir/ap[ru]_*.h"
>> >  fi
>> >
>> > +CPPFLAGS="`echo $CPPFLAGS`"
>> > +cppflags="`echo $cppflags`"
>>
>> Why worry about extra spaces? What problem was it causing? This code
>> is spurious, unless there was a problem caused by those spaces.
>
> I want to avoid some spaces in generated csvn/core/functions.py.
>
> --
> Arfrever Frehtes Taifersar Arahesis
>

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2023152


Re: svn commit: r37531 - in trunk: . build

Posted by Arfrever Frehtes Taifersar Arahesis <Ar...@GMail.Com>.
2009-05-02 02:22:26 Greg Stein napisał(a):
> On Sat, May 2, 2009 at 01:46, Arfrever Frehtes Taifersar Arahesis
> <Ar...@gmail.com> wrote:
> >...
> > +++ trunk/build/run_ctypesgen.sh        Fri May  1 16:46:38 2009        (r37531)
> > @@ -13,15 +13,18 @@ CTYPESGEN="$5"
> >  abs_srcdir="$6"
> >  abs_builddir="$7"
> >
> > -svn_prefix="$8"
> > +svn_libdir="$8"
> >  apr_prefix="$9"
> >  apu_prefix="${10}"
> >
> >  cp_relpath="subversion/bindings/ctypes-python"
> > -### tempfile instead?
> > -output="$abs_builddir/$cp_relpath/svn_all.py"
> > +output="$cp_relpath/svn_all.py"
> 
> Why this change? The abs_builddir works just fine, so what are you
> trying to accomplish here?

I don't want to see build path in generated csvn/core/functions.py.
subversion/bindings/ctypes-python/setup.py doesn't add build path to
csvn/core/functions.py.

> > -svn_includes="$abs_srcdir/subversion/include"
> > +if test "$abs_builddir" = "$abs_srcdir"; then
> > +  svn_includes="subversion/include"
> > +else
> > +  svn_includes="$abs_srcdir/subversion/include"
> > +fi
> 
> Why this change? The abs_srcdir works, so there is never a need to
> omit it. This "if" statement is just extra complexity.

I don't want to see build path in generated csvn/core/functions.py.
subversion/bindings/ctypes-python/setup.py doesn't add build path to
csvn/core/functions.py.

> >...
> >  # This order is important. The resulting stubs will load libraries in
> > @@ -54,6 +57,8 @@ if test "$apr_include_dir" != "$apu_incl
> >   includes="$includes $apu_include_dir/ap[ru]_*.h"
> >  fi
> >
> > +CPPFLAGS="`echo $CPPFLAGS`"
> > +cppflags="`echo $cppflags`"
> 
> Why worry about extra spaces? What problem was it causing? This code
> is spurious, unless there was a problem caused by those spaces.

I want to avoid some spaces in generated csvn/core/functions.py.

-- 
Arfrever Frehtes Taifersar Arahesis