You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by kf...@collab.net on 2003/06/10 15:10:04 UTC

Re: svn commit: rev 6179 - in trunk: . build/ac-macros

patrick@tigris.org writes:
> New Revision: 6179
> 
> Modified:
>    trunk/build/ac-macros/neon.m4
>    trunk/svn-config.in
> Log:
> Fix for Issue 751. (references to abs_srcdir and abs_builddir in
> svn-config)

Patrick, I don't want to stand in the way of work getting done, but
should point out that your commit access is for the Java bindings.
The limitation is not because anyone thinks badly of your coding, just
that we hadn't seen enough patches from you to say "Give this guy full
commit access" :-).

Committing directly to other areas is still okay, but the log message
should say "Reviewed by so-and-so", where so-and-so is some committer
with expertise in the relevant domain.  (Possibly that happened in
this case, and you just forgot to mention it in the log message?)

Or, maybe some committer wants to review it after the fact, which
would also be fine.  Doing things in commit-then-review order is okay,
as long as *someone* is committed to reviewing it...

That's all -- not a big deal, and I'm certainly not asking you to
revert it or anything.  Just pointing it out for next time.

(I don't have time/competence to review it myself right now,
unfortunately.)

By the way, were you aware that as of rev 5859, svn-config is not
installed?  So your change won't have any effect by itself.

-Karl

>  * build/ac-macros/neon.m4
> 	added SVN_NEON_INCLUDES_SVN_CONFIG for use in svn-config script.
> 	added NEON_LIBS_SVN_CONFIG for use in svn-config script.
> 	SVN_NEON_INCLUDES and NEON_LIBS were used for this purpose.
> 	These are also used for Makefile which has a different syntax.
> 
> * svn-config.in
> 	changed SVN_NEON_INCLUDES to SVN_NEON_INCLUDES_SVN_CONFIG.
> 	changed NEON_LIBS to NEON_LIBS_SVN_CONFIG.
> 
> 
> 
> Modified: trunk/build/ac-macros/neon.m4
> ==============================================================================
> --- trunk/build/ac-macros/neon.m4	(original)
> +++ trunk/build/ac-macros/neon.m4	Tue Jun 10 00:05:08 2003
> @@ -53,6 +53,8 @@
>            echo "Using neon found in source directory."
>            SVN_NEON_INCLUDES=-'I$(abs_srcdir)/neon/src'
>            NEON_LIBS="\$(abs_builddir)/neon/src/libneon.la"
> +          SVN_NEON_INCLUDES_SVN_CONFIG="-I$abs_srcdir/neon/src"
> +          NEON_LIBS_SVN_CONFIG="$abs_builddir/neon/src/libneon.la"
>  
>  dnl Configure neon --------------------------
>            # The arguments passed to this configure script are passed down to
> @@ -85,8 +87,11 @@
>              changequote(<<, >>)dnl
>              CFLAGS="$CFLAGS `$SHELL $abs_builddir/neon/neon-config --cflags | sed -e 's/-I[^ ]*//g'`"
>              SVN_NEON_INCLUDES="$SVN_NEON_INCLUDES `$SHELL $abs_builddir/neon/neon-config --cflags | sed -e 's/-D[^ ]*//g'`"
> -            changequote([, ])dnl
>              svn_lib_neon="yes"
> +            NEON_LIBS_SVN_CONFIG="$NEON_LIBS_SVN_CONFIG $NEON_LIBS_NEW"
> +            SVN_NEON_INCLUDES_SVN_CONFIG="$SVN_NEON_INCLUDES_SVN_CONFIG `$SHELL $abs_builddir/neon/neon-config --cflags | sed -e 's/-D[^ ]*//g'`"
> +            changequote([, ])dnl
> +	
>            fi
>  
>            SVN_SUBDIRS="$SVN_SUBDIRS neon"
> @@ -108,6 +113,8 @@
>    
>    AC_SUBST(SVN_NEON_INCLUDES)
>    AC_SUBST(NEON_LIBS)
> +  AC_SUBST(SVN_NEON_INCLUDES_SVN_CONFIG)
> +  AC_SUBST(NEON_LIBS_SVN_CONFIG)
>  ])
>  
>  dnl SVN_NEON_CONFIG()
> 
> Modified: trunk/svn-config.in
> ==============================================================================
> --- trunk/svn-config.in	(original)
> +++ trunk/svn-config.in	Tue Jun 10 00:05:08 2003
> @@ -22,10 +22,10 @@
>  libdir="@libdir@"
>  includedir="@includedir@"
>  
> -LIBS="@NEON_LIBS@ @SVN_APRUTIL_EXPORT_LIBS@ @SVN_APR_EXPORT_LIBS@ @SVN_DB_LIBS@ @LIBS@"
> +LIBS="@NEON_LIBS_SVN_CONFIG@ @SVN_APRUTIL_EXPORT_LIBS@ @SVN_APR_EXPORT_LIBS@ @SVN_DB_LIBS@ @LIBS@"
>  CFLAGS="@CFLAGS@"
>  CPPFLAGS="@CPPFLAGS@"
> -INCLUDES="@SVN_NEON_INCLUDES@ @SVN_DB_INCLUDES@ @SVN_APR_INCLUDES@ @SVN_APRUTIL_INCLUDES@"
> +INCLUDES="@SVN_NEON_INCLUDES_SVN_CONFIG@ @SVN_DB_INCLUDES@ @SVN_APR_INCLUDES@ @SVN_APRUTIL_INCLUDES@"
>  LDFLAGS="@LDFLAGS@"
>  
>  SVN_SOURCE_DIR="@abs_srcdir@"
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

RE: svn commit: rev 6179 - in trunk: . build/ac-macros

Posted by Sander Striker <st...@apache.org>.
> From: Patrick Mayweg [mailto:mayweg@qint.de]
> Sent: Wednesday, June 11, 2003 10:15 AM

>> My point is that your 'intermediary' step introduces a bogus change into the
>> main build system.
> 
> Will you or should backout my changes?

Mostly the committer of the 'bogus' change backs out his changes
himself after negative review.  However, if that takes to long to
happen someone will grudgingly do it for him.  At least, that's
how it works in httpd land, and since we tend to do things the
same over here...


Sander

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: rev 6179 - in trunk: . build/ac-macros

Posted by Patrick Mayweg <ma...@qint.de>.
Hi Justin,

Justin Erenkrantz wrote:

> --On Wednesday, June 11, 2003 8:00 AM +0200 Patrick Mayweg <ma...@qint.de>
> wrote:
>
> > What a frequency of reposts do you think is appropriate ? Daily, weekly or
> > monthly ?
>
> Personally, I recommend once every two weeks until you get a reply.  I think
> some other people here on dev@svn have advocated once a week.  Circumstances
> change enough with people every two weeks that I think it's a relatively safe
> guide.  That said, it still may take a few tries to reach someone especially
> if it's a relatively arcane topic where there are relatively few eyes (i.e.
> build system).

That gives me a time frame for the integration into the buildsystem.

>
>
> > abs_srcdir and abs_builddir are not known in svn-config. I could have
> > introduced them to the svn-config but it would have work alone, because the
> > syntax for variables in a shell script is different than for a Makefile.
> >
> > What is what svn-config was meant for ? I thought it meant to give you the
> > include path and the libraries needed, when you want to build an
> > appilication on top of the subversion libraries. If you take a subversion
> > release tarball, you will get the abs_srcdir and the abs_builddir problem
> > and the resulting Makefiles have to be edited manualy.
>
> The problem is that svn-config is meant to work on installed copies of
> Subversion.  That means that it can not reference anything withing abs_srcdir
> or abs_builddir as we can not assume that those still exist when svn-config is
> finally invoked.  So, my point is that those references to abs_srcdir whether
> explicit via $(abs_srcdir) or expanded at configure-time via $abs_srcdir still
> don't work.
>
> svn-config doesn't work right now and isn't installed on purpose because it is
> fundamentally broken if you use in-tree dependencies.  If you don't use
> in-tree dependencies, then it works just fine.  But, your commit just attempts
> to hide the problem rather than dealing with it.  I'd really rather not do
> that.

I have looked into my configure script and I will forget about svn-config for the
javahl-binding. About for svnup it is not a nice outlook. I will have to maintain
the jni-binding in there until the integration is atleast partialy done. And
without svn-config, how do I best find out where to find the include files and the
libraries ?

>
>
> > This is what I want to do. I just started with what I already had and
> > adopted it just as an intermediate step. To integrate into the build
> > systems, I have to understand them first. But since I have to process
> > different kind of files (java source and class files) as the rest of
> > subversion, it may mean a major extentsion to the buildsystems.
>
> My point is that your 'intermediary' step introduces a bogus change into the
> main build system.

Will you or should backout my changes?

>
>
> > I did not had the time to try to understand the buildsystems. I also need to
> > create the dsp and ?vcproj? files on windows.
>
> Yup.  That's why if you can integrate with the build.conf, you'll get those
> for free.  But, that's something that Greg has the most experience with.
> You'll probably need some Python-fu in order to understand it.

Who can I ask about build.conf? A first look tells me I have to understand
gen-make.py to add my things to it. To add the java things to build.conf,
gen-make.pk and its friends, that will take some time. Each target system has it
own python class.

>

>
>
> > This is only the intermediate step, I will remove automake, when I
> > intergrate with the build system.
>
> Yup, but if you want the top-level configure script to produce the Makefile,
> then it can't have any automake artifacts.  I think that's the easiest
> short-term solution available to you.  If you have time later, then you can
> figure out how to get it to work with build.conf.

I do not think that this step is worth the effort. Manualy removing the
Makefile.in from all automake things and integrating into the top-level configure
script will take me 2 weeks and the patch into configure.in another month till it
is reviewed. I better spend my time looking into gen-make.py and friends.

>

>
>
> > What is a pkg-config file ? Where do I find information about that ?
>
> <http://www.freedesktop.org/software/pkgconfig/>  -- justin

Patrick


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: rev 6179 - in trunk: . build/ac-macros

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Wednesday, June 11, 2003 8:00 AM +0200 Patrick Mayweg <ma...@qint.de> 
wrote:

> What a frequency of reposts do you think is appropriate ? Daily, weekly or
> monthly ?

Personally, I recommend once every two weeks until you get a reply.  I think 
some other people here on dev@svn have advocated once a week.  Circumstances 
change enough with people every two weeks that I think it's a relatively safe 
guide.  That said, it still may take a few tries to reach someone especially 
if it's a relatively arcane topic where there are relatively few eyes (i.e. 
build system).

> abs_srcdir and abs_builddir are not known in svn-config. I could have
> introduced them to the svn-config but it would have work alone, because the
> syntax for variables in a shell script is different than for a Makefile.
>
> What is what svn-config was meant for ? I thought it meant to give you the
> include path and the libraries needed, when you want to build an
> appilication on top of the subversion libraries. If you take a subversion
> release tarball, you will get the abs_srcdir and the abs_builddir problem
> and the resulting Makefiles have to be edited manualy.

The problem is that svn-config is meant to work on installed copies of 
Subversion.  That means that it can not reference anything withing abs_srcdir 
or abs_builddir as we can not assume that those still exist when svn-config is 
finally invoked.  So, my point is that those references to abs_srcdir whether 
explicit via $(abs_srcdir) or expanded at configure-time via $abs_srcdir still 
don't work.

svn-config doesn't work right now and isn't installed on purpose because it is 
fundamentally broken if you use in-tree dependencies.  If you don't use 
in-tree dependencies, then it works just fine.  But, your commit just attempts 
to hide the problem rather than dealing with it.  I'd really rather not do 
that.

> This is what I want to do. I just started with what I already had and
> adopted it just as an intermediate step. To integrate into the build
> systems, I have to understand them first. But since I have to process
> different kind of files (java source and class files) as the rest of
> subversion, it may mean a major extentsion to the buildsystems.

My point is that your 'intermediary' step introduces a bogus change into the 
main build system.

> I did not had the time to try to understand the buildsystems. I also need to
> create the dsp and ?vcproj? files on windows.

Yup.  That's why if you can integrate with the build.conf, you'll get those 
for free.  But, that's something that Greg has the most experience with. 
You'll probably need some Python-fu in order to understand it.

> This is only the intermediate step, I will remove automake, when I
> intergrate with the build system.

Yup, but if you want the top-level configure script to produce the Makefile, 
then it can't have any automake artifacts.  I think that's the easiest 
short-term solution available to you.  If you have time later, then you can 
figure out how to get it to work with build.conf.

> What is a pkg-config file ? Where do I find information about that ?

<http://www.freedesktop.org/software/pkgconfig/>  -- justin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: rev 6179 - in trunk: . build/ac-macros

Posted by Patrick Mayweg <ma...@qint.de>.
Hi Justin,

Justin Erenkrantz wrote:

> --On Wednesday, June 11, 2003 6:27 AM +0200 Patrick Mayweg <ma...@qint.de>
> wrote:
>
> > I am sorry that you feal that way. I have posted the modifications twice to
> > the mail list. But nobody cared to answer. This patch is needed to configure
> > the java-bindings.
>
> The fact that no one 'cared to answer' does not mean that you should commit
> without prior review.  We all are extremely busy and this is a high-traffic
> list.  Some posts just slip through.  You should keep reposting until someone
> replies, but please don't assume that silence equals review and assent.
> Please be patient though.  (Two posts in less than two weeks isn't a long
> time.)

What a frequency of reposts do you think is appropriate ? Daily, weekly or monthly
?

>
>
> That said, I believe that your patch/commit is incorrect as it tries to avoid
> the problem rather than resolve the issue raised in 751.  It merely expands
> the abs_srcdir and abs_builddir at configure-time rather than teaching
> svn-config how to deal with installed copies of our dependencies.  So, it
> doesn't serve to resolve the end problem as discussed in 751, but merely adds
> a layer of indirection that doesn't help anyone.  It'll only work for your
> specific edge-case, but your edge-case isn't at all what svn-config was meant
> for.

abs_srcdir and abs_builddir are not known in svn-config. I could have introduced
them to the svn-config but it would have work alone, because the syntax for
variables in a shell script is different than for a Makefile.

What is what svn-config was meant for ? I thought it meant to give you the include
path and the libraries needed, when you want to build an appilication on top of the
subversion libraries. If you take a subversion release tarball, you will get the
abs_srcdir and the abs_builddir problem and the resulting Makefiles have to be
edited manualy.

>
> In your particular case, the correct solution is to integrate your Java
> bindings into the Subversion build system.  The top-level configure script has
> the knowledge of the variables needed to handle the location of the libraries.
> Therefore, you should submit patches to integrate those bindings into the main
> build system.

This is what I want to do. I just started with what I already had and adopted it
just as an intermediate step. To integrate into the build systems, I have to
understand them first. But since I have to process different kind of files (java
source and class files) as the rest of subversion, it may mean a major extentsion
to the buildsystems.

>
>
> You could either add the required logic to build.conf, or I'd personally be
> okay with having autoconf create the Makefile in the subversion/bindings/java
> directory.

I did not had the time to try to understand the buildsystems. I also need to create
the dsp and ?vcproj? files on windows.

>
>
> I also noticed that you have introduced a dependency of automake into your
> bindings.  So, you will have to remove those as the core of Subversion itself
> does not use automake.

This is only the intermediate step, I will remove automake, when I intergrate with
the build system.

>
>
> > svn-config does not need to be installed for the patch to work  in the
> > current form.
>
> The problem is that svn-config wasn't meant to solve the problem of something
> in-tree needing the variables.  The correct solution in your case is to
> generate the Makefiles from the top-level configure.
>
> I have a hunch that it *may* be better to scuttle svn-config and just create a
> pkg-config file.  I'm not a great fan of pkg-config, but it may be able to
> solve the dependency problems in a way that is hard to do otherwise.  -- justin

What is a pkg-config file ? Where do I find information about that ?

Regards,
Patrick


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: rev 6179 - in trunk: . build/ac-macros

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Wednesday, June 11, 2003 6:27 AM +0200 Patrick Mayweg <ma...@qint.de> 
wrote:

> I am sorry that you feal that way. I have posted the modifications twice to
> the mail list. But nobody cared to answer. This patch is needed to configure
> the java-bindings.

The fact that no one 'cared to answer' does not mean that you should commit 
without prior review.  We all are extremely busy and this is a high-traffic 
list.  Some posts just slip through.  You should keep reposting until someone 
replies, but please don't assume that silence equals review and assent. 
Please be patient though.  (Two posts in less than two weeks isn't a long 
time.)

That said, I believe that your patch/commit is incorrect as it tries to avoid 
the problem rather than resolve the issue raised in 751.  It merely expands 
the abs_srcdir and abs_builddir at configure-time rather than teaching 
svn-config how to deal with installed copies of our dependencies.  So, it 
doesn't serve to resolve the end problem as discussed in 751, but merely adds 
a layer of indirection that doesn't help anyone.  It'll only work for your 
specific edge-case, but your edge-case isn't at all what svn-config was meant 
for.

In your particular case, the correct solution is to integrate your Java 
bindings into the Subversion build system.  The top-level configure script has 
the knowledge of the variables needed to handle the location of the libraries. 
Therefore, you should submit patches to integrate those bindings into the main 
build system.

You could either add the required logic to build.conf, or I'd personally be 
okay with having autoconf create the Makefile in the subversion/bindings/java 
directory.

I also noticed that you have introduced a dependency of automake into your 
bindings.  So, you will have to remove those as the core of Subversion itself 
does not use automake.

> svn-config does not need to be installed for the patch to work  in the
> current form.

The problem is that svn-config wasn't meant to solve the problem of something 
in-tree needing the variables.  The correct solution in your case is to 
generate the Makefiles from the top-level configure.

I have a hunch that it *may* be better to scuttle svn-config and just create a 
pkg-config file.  I'm not a great fan of pkg-config, but it may be able to 
solve the dependency problems in a way that is hard to do otherwise.  -- justin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: rev 6179 - in trunk: . build/ac-macros

Posted by kf...@collab.net.
Patrick Mayweg <ma...@qint.de> writes:
> > Patrick, I don't want to stand in the way of work getting done, but
> > should point out that your commit access is for the Java bindings.
> > The limitation is not because anyone thinks badly of your coding, just
> > that we hadn't seen enough patches from you to say "Give this guy full
> > commit access" :-).
> 
> I am sorry that you feal that way. I have posted the modifications
> twice to the mail list. But nobody cared to answer. This patch is
> needed to configure the java-bindings. 

Uh, that restriction is not "how I feel", Patrick, it's how we work.
I'm just the messenger here.  The fact that your post didn't get a
response doesn't mean that someone approved the commit -- if someone
consciously approved it, they'd take the ten seconds to say so!  No,
it just means that no one had time to review it.

In this case, the best solution is pretty simple, though -- next time,
your log message should say "This patch is needed to configure the
java bindings."  Then people would have known that it was part of Java
bindings maintenance.

I see that you and Justin are discussing what to do, so the technical
questions will get sorted out one way or the other.

My only request is that if you make a commit that's not obviously
about Java bindings, then please say in the log message how it relates
to the bindings, so people don't need to wonder if you're suddenly
committing all over the codebase for non-Java issues (in other words,
a change in the type of your commit access, for which we have a
process -- and, by the way, good reasons for that process).

Thanks,
-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: rev 6179 - in trunk: . build/ac-macros

Posted by Patrick Mayweg <ma...@qint.de>.
Hi Karl,

kfogel@collab.net wrote:

> patrick@tigris.org writes:
> > New Revision: 6179
> >
> > Modified:
> >    trunk/build/ac-macros/neon.m4
> >    trunk/svn-config.in
> > Log:
> > Fix for Issue 751. (references to abs_srcdir and abs_builddir in
> > svn-config)
>
> Patrick, I don't want to stand in the way of work getting done, but
> should point out that your commit access is for the Java bindings.
> The limitation is not because anyone thinks badly of your coding, just
> that we hadn't seen enough patches from you to say "Give this guy full
> commit access" :-).

I am sorry that you feal that way. I have posted the modifications twice to the mail list. But nobody cared to answer. This patch is needed to configure
the java-bindings.

>
>
> Committing directly to other areas is still okay, but the log message
> should say "Reviewed by so-and-so", where so-and-so is some committer
> with expertise in the relevant domain.  (Possibly that happened in
> this case, and you just forgot to mention it in the log message?)

I am more than happy, if anybody who has expertise in this area would like to review my changes.

>
>
> Or, maybe some committer wants to review it after the fact, which
> would also be fine.  Doing things in commit-then-review order is okay,
> as long as *someone* is committed to reviewing it...

How do I find someone to review my changes, if nobody answers to my mail to the mailing list ?

>
>
> That's all -- not a big deal, and I'm certainly not asking you to
> revert it or anything.  Just pointing it out for next time.
>
> (I don't have time/competence to review it myself right now,
> unfortunately.)
>
> By the way, were you aware that as of rev 5859, svn-config is not
> installed?  So your change won't have any effect by itself.

svn-config does not need to be installed for the patch to work  in the current form.

>
>
> -Karl
>

Regards,
Patrick


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org