[PATCH] README: Replace "configure" with "autogen.sh" for correct installation

Yi Wang posted 1 patch 1 week ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1589532688-39853-1-git-send-email-wang.yi59@zte.com.cn
README.rst | 4 ++--
autogen.sh | 4 ++++
2 files changed, 6 insertions(+), 2 deletions(-)

[PATCH] README: Replace "configure" with "autogen.sh" for correct installation

Posted by Yi Wang 1 week ago
From: LiaoPingfang <liao.pingfang@zte.com.cn>

First of all, there is no "configure" file.
We need use autogen.sh at first to create "configure" file.
Then in autogen.sh "configure" will be executed according to
user as root or non-root.

Signed-off-by: Liao Pingfang <liao.pingfang@zte.com.cn>
---
 README.rst | 4 ++--
 autogen.sh | 4 ++++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/README.rst b/README.rst
index cdcc97b..82b4b31 100644
--- a/README.rst
+++ b/README.rst
@@ -53,7 +53,7 @@ in a manner that is suitable for installing as root, use:
 ::
 
   $ mkdir build && cd build
-  $ ../configure --prefix=/usr --sysconfdir=/etc --localstatedir=/var
+  $ ../autogen.sh
   $ make
   $ sudo make install
 
@@ -62,7 +62,7 @@ While to build & install as an unprivileged user
 ::
 
   $ mkdir build && cd build
-  $ ../configure --prefix=$HOME/usr
+  $ ../autogen.sh
   $ make
   $ make install
 
diff --git a/autogen.sh b/autogen.sh
index 4e1bbce..3c119e1 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -27,6 +27,10 @@ if test "x$1" = "x--system"; then
       libdir=$prefix/lib64
     fi
     EXTRA_ARGS="--prefix=$prefix --sysconfdir=$sysconfdir --localstatedir=$localstatedir --libdir=$libdir"
+    if [ "$HOME" != "/root" ]; then
+	    prefix=$HOME/usr
+	    EXTRA_ARGS="--prefix=$prefix"
+    fi
 fi
 
 cd "$olddir"
-- 
2.9.5

Re: [PATCH] README: Replace "configure" with "autogen.sh" for correct installation

Posted by Peter Krempa 1 week ago
On Fri, May 15, 2020 at 16:51:28 +0800, Yi Wang wrote:
> From: LiaoPingfang <liao.pingfang@zte.com.cn>
> 
> First of all, there is no "configure" file.
> We need use autogen.sh at first to create "configure" file.
> Then in autogen.sh "configure" will be executed according to
> user as root or non-root.

NACK, autogen needs to be run when running from a git checkout, but is
not present when building from a tarball.

You can modify that statement but blankly stating that autogen needs to
be run is wrong.

> 
> Signed-off-by: Liao Pingfang <liao.pingfang@zte.com.cn>
> ---

Re: [PATCH] README: Replace "configure" with "autogen.sh" for correct installation

Posted by Michal Privoznik 1 week ago
On 5/15/20 11:01 AM, Peter Krempa wrote:
> On Fri, May 15, 2020 at 16:51:28 +0800, Yi Wang wrote:
>> From: LiaoPingfang <liao.pingfang@zte.com.cn>
>>
>> First of all, there is no "configure" file.
>> We need use autogen.sh at first to create "configure" file.
>> Then in autogen.sh "configure" will be executed according to
>> user as root or non-root.
> 
> NACK, autogen needs to be run when running from a git checkout, but is
> not present when building from a tarball.
> 
> You can modify that statement but blankly stating that autogen needs to
> be run is wrong.

Well, I guess we need two sections. Thing is, README serves two goals 
since we switched to gitlab. The first one, the old one - README as part 
of release tarball, but it also gained new purpose - landing webpage for 
developers who browse gitlab (that is why we have images in the .rst).

Therefore, what we should do is to have a section that covers "building 
from git" and another one "building from a tarball". Note that we 
already cover these in [1], so maybe after all drop all compile info in 
favour of the link?

Michal

1: https://libvirt.org/compiling.html

Re: [PATCH] README: Replace "configure" with "autogen.sh" for correct installation

Posted by Daniel P. Berrangé 1 week ago
On Fri, May 15, 2020 at 11:09:50AM +0200, Michal Privoznik wrote:
> On 5/15/20 11:01 AM, Peter Krempa wrote:
> > On Fri, May 15, 2020 at 16:51:28 +0800, Yi Wang wrote:
> > > From: LiaoPingfang <liao.pingfang@zte.com.cn>
> > > 
> > > First of all, there is no "configure" file.
> > > We need use autogen.sh at first to create "configure" file.
> > > Then in autogen.sh "configure" will be executed according to
> > > user as root or non-root.
> > 
> > NACK, autogen needs to be run when running from a git checkout, but is
> > not present when building from a tarball.
> > 
> > You can modify that statement but blankly stating that autogen needs to
> > be run is wrong.
> 
> Well, I guess we need two sections. Thing is, README serves two goals since
> we switched to gitlab. The first one, the old one - README as part of
> release tarball, but it also gained new purpose - landing webpage for
> developers who browse gitlab (that is why we have images in the .rst).
> 
> Therefore, what we should do is to have a section that covers "building from
> git" and another one "building from a tarball". Note that we already cover
> these in [1], so maybe after all drop all compile info in favour of the
> link?

Or just ignore the problem for now, since when we switch to Meson, we'll
have the same instructions regardless of whether building from git or
tarballs.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] README: Replace "configure" with "autogen.sh" for correct installation

Posted by Michal Privoznik 1 week ago
On 5/15/20 11:13 AM, Daniel P. Berrangé wrote:
> Or just ignore the problem for now, since when we switch to Meson, we'll
> have the same instructions regardless of whether building from git or
> tarballs.
> 

I'm not strictly against that, but I guess that might take a while. 
Pavel? And this is clearly a feedback from somebody who is not a core 
developer, that our build instructions are not as clear as they might be.

Michal

Re: [PATCH] README: Replace "configure" with "autogen.sh" for correct installation

Posted by Pavel Hrdina 1 week ago
On Fri, May 15, 2020 at 12:03:01PM +0200, Michal Privoznik wrote:
> On 5/15/20 11:13 AM, Daniel P. Berrangé wrote:
> > Or just ignore the problem for now, since when we switch to Meson, we'll
> > have the same instructions regardless of whether building from git or
> > tarballs.
> > 
> 
> I'm not strictly against that, but I guess that might take a while. Pavel?
> And this is clearly a feedback from somebody who is not a core developer,
> that our build instructions are not as clear as they might be.

It will take few more weeks I guess.  I'm just finishing the src/
directory, the following once will not take that long, hopefully.

Improving current documentation makes sense as the meson rewrite will
just replace most of the commands and possible removes/adds a new once.

Pavel

Re: [PATCH] README: Replace "configure" with "autogen.sh" for correct installation

Posted by Daniel P. Berrangé 1 week ago
On Fri, May 15, 2020 at 12:03:01PM +0200, Michal Privoznik wrote:
> On 5/15/20 11:13 AM, Daniel P. Berrangé wrote:
> > Or just ignore the problem for now, since when we switch to Meson, we'll
> > have the same instructions regardless of whether building from git or
> > tarballs.
> > 
> 
> I'm not strictly against that, but I guess that might take a while. Pavel?
> And this is clearly a feedback from somebody who is not a core developer,
> that our build instructions are not as clear as they might be.

True, lets just remove the instructions from README.rst, and add a line
that says

   "For instructions on building libvirt see docs/compiling.rst"

and convert compiling.html.in to RST so that it is pleasant to read

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|