The allfaces.c file is mentioned in the cddlib manual as an example file. Its been in the sage spkg for a long time, I think. Probably it was originally included to show sage developers how to use the library. So it seems to make sense to get rid of it after #8115 is sorted out, since cdd_both_reps provides an example that is actually used.
My apologies for missing this issue in review - makefiles and libraries are not my strong suit. -Marshall On Jan 29, 7:11 am, Volker Braun <vbraun.n...@gmail.com> wrote: > The cdd_both_reps/cdd_both_reps_gmp are used by the new polyhedra > package, and are indeed introduced in cddlib-094f.p2.spkg. > > I have no idea what the patches/allfaces.c is for - it does not part > of the cddlib library, but only some utility binary. As far as I can > tell the allfaces binary is never used by sage. Maybe whoever added it > can shed some light on this? Its not documented in the SPKG.txt, sigh. > > As for the patches/cdd_both_reps-make.patch, this only records the > changes to the automake source files. You still need to run autoconf/ > automake to generate the makefiles from that. I still don't know the > correct way of handling this: > * The automake sources might be automake-version dependent. > * Not everyone has all versions of automake installed, so spkg- > install can't call automake. > * Running autoconf/automake generates big shell scripts (configure, > makefile). Differences in these need not be recorded. > * But different versions of automake will produce different scripts, > which would clutter up naive patches. > See also:http://trac.sagemath.org/sage_trac/ticket/8079 > > I didn't realize that /src is supposed to be the pristine upstream > source. Note that I previously entered a cddlib-094f.p3.spkg into trac > for review athttp://trac.sagemath.org/sage_trac/ticket/3304to build > cddlib as a shared library as well as a couple of other improvements. > I'll try to merge the changes from your .p3 with mine and report back. > > Best wishes, > Volker > > On Jan 29, 10:09 am, Minh Nguyen <nguyenmi...@gmail.com> wrote: > > > Hi folks, > > > With Sage 4.3.2.alpha0, one has two suspicious binary files under > > SAGE_LOCAL/bin, as shown by the following hg report: > > > [mv...@sage bin]$ pwd > > /dev/shm/mvngu/sage-4.3.2.alpha0-sage.math/local/bin > > [mv...@sage bin]$ hg status > > M sage-banner > > M sage-gdb-commands > > M sage-maxima.lisp > > M sage-verify-pyc > > ? cdd_both_reps > > ? cdd_both_reps_gmp > > > The files in question are cdd_both_reps and cdd_both_reps_gmp. It > > turns out that these two files were put there by the package > > cddlib-094f.p2.spkg. I think they were put there due to using the > > command "patch" in spkg-install for patching the upstream source under > > src/. > > > A problem with cddlib-094f.p2.spkg is that it patches upstream source > > using a patch file, rather than copying a patched file over to the > > appropriate place under the src/ directory. Consequently, there is no > > clean separation between upstream source and patches that we apply to > > cddlib-094f. Looking at spkg-install of cddlib-094f.p2.spkg, you get > > these two lines for applying Sage-specific patches: > > > cp patches/allfaces.c src/src/ > > > patch -p0 < patches/cdd_both_reps-make.patch > > > The first line is the preferred way to patch an upstream source > > because it copies the patched file patches/allfaces.c over to > > src/src/. Even before running the installation script spkg-install, > > the original upstream source is the file src/src/allfaces.c and its > > corresponding patched version for Sage is patches/allfaces.c. And you > > patch the upstream source using cp. In this way, you keep a clean > > separation between the source provided by the upstream project and the > > patches that the Sage project applies on top of the upstream source. > > > Now let's consider the second line in the above code listing, i.e. > > > patch -p0 < patches/cdd_both_reps-make.patch > > > Patching an upstream source in this way results in modified upstream > > source and this modification now lives under src/, which is supposedly > > where the clean upstream source lives. Subsequently, anytime you want > > to update (not upgrade to latest upstream release) the > > cddlib-094f.p2.spkg package, you have modified source under src/. This > > can cause a lot of confusion for maintainers who need to have a clean > > separation between upstream source and Sage-specific patches. > > > Here's an example illustrating that using the command "patch" in > > spkg-install for patching upstream source is a bad idea. Say you have > > a directory listing as follows: > > > [mv...@sage cddlib]$ pwd > > /home/mvngu/spkg/standard/cddlib > > [mv...@sage cddlib]$ ls > > cddlib-094f.p0 cddlib-094f.p0.spkg cddlib-094f.p2 cddlib-094f.p2.spkg > > > In cddlib-094f.p2.spkg, the content of the file > > patches/cdd_both_reps-make.patch is > > > [mv...@sage cddlib]$ cat cddlib-094f.p2/patches/cdd_both_reps-make.patch > > --- ../cddlib-094f/src/src-gmp/Makefile.am 2009-01-26 > > 09:30:16.000000000 +0000 > > +++ src/src-gmp/Makefile.am 2009-10-04 10:36:17.000000000 +0100 > > @@ -11,7 +11,8 @@ > > testcdd2_gmp \ > > testlp1_gmp \ > > testlp2_gmp \ > > -testlp3_gmp > > +testlp3_gmp \ > > +cdd_both_reps_gmp > > #cddmathlink > > > scdd_gmp_SOURCES = simplecdd.c > > @@ -27,6 +28,7 @@ > > testlp1_gmp_SOURCES = testlp1.c > > testlp2_gmp_SOURCES = testlp2.c > > testlp3_gmp_SOURCES = testlp3.c > > +cdd_both_reps_gmp_SOURCES = cdd_both_reps.c > > # cddmathlink_SOURCES = cddmathlink.c cddmlio.h cddmlio.c > > > LDADD = ../lib-src-gmp/libcddgmp.a > > --- ../cddlib-094f/src/src/Makefile.am 2009-01-26 09:30:03.000000000 +0000 > > +++ src/src/Makefile.am 2009-10-04 10:36:17.000000000 +0100 > > @@ -11,7 +11,8 @@ > > testcdd2 \ > > testlp1 \ > > testlp2 \ > > -testlp3 > > +testlp3 \ > > +cdd_both_reps > > #cddmathlink > > > scdd_SOURCES = simplecdd.c > > @@ -27,6 +28,7 @@ > > testlp1_SOURCES = testlp1.c > > testlp2_SOURCES = testlp2.c > > testlp3_SOURCES = testlp3.c > > +cdd_both_reps_SOURCES = cdd_both_reps.c > > # cddmathlink_SOURCES = cddmathlink.c cddmlio.h cddmlio.c > > > LDADD = ../lib-src/libcdd.a > > > Now compare cddlib-094f.p0/src/src-gmp/Makefile.am with > > cddlib-094f.p2/src/src-gmp/Makefile.am and you'd get > > > [mv...@sage cddlib]$ diff -u cddlib-094f.p0/src/src-gmp/Makefile.am > > cddlib-094f.p2/src/src-gmp/Makefile.am--- > > cddlib-094f.p0/src/src-gmp/Makefile.am 2009-01-26 01:30:16.000000000 > > -0800 > > +++ cddlib-094f.p2/src/src-gmp/Makefile.am 2009-10-15 > > 13:25:26.000000000 -0700 > > @@ -11,7 +11,8 @@ > > testcdd2_gmp \ > > testlp1_gmp \ > > testlp2_gmp \ > > -testlp3_gmp > > +testlp3_gmp \ > > +cdd_both_reps_gmp > > #cddmathlink > > > scdd_gmp_SOURCES = simplecdd.c > > @@ -27,6 +28,7 @@ > > testlp1_gmp_SOURCES = testlp1.c > > testlp2_gmp_SOURCES = testlp2.c > > testlp3_gmp_SOURCES = testlp3.c > > +cdd_both_reps_gmp_SOURCES = cdd_both_reps.c > > # cddmathlink_SOURCES = cddmathlink.c cddmlio.h cddmlio.c > > > LDADD = ../lib-src-gmp/libcddgmp.a > > > As you can see, cddlib-094f.p2.spkg is mashing up upstream source and > > Sage-specific patch together and put the result under the directory > > where the clean upstream, and only the clean upstream source, should > > live. This is a recipe for confusion. An updated cddlib spkg is up at > > >http://trac.sagemath.org/sage_trac/ticket/8115 > > > awaiting review. On Sage 4.3.1, forcing a re-installation of the > > updated cddlib spkg did not result in the above binary files copied > > over to SAGE_LOCAL/bin. > > > -- > > Regards > > Minh Van Nguyen -- To post to this group, send an email to sage-devel@googlegroups.com To unsubscribe from this group, send an email to sage-devel+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/sage-devel URL: http://www.sagemath.org
