Opened 7 years ago

Closed 6 years ago

#13245 closed defect (fixed)

sage --clone: do not rebuild the entire Sage library, and do not rebuild the docs

Reported by: jhpalmieri Owned by: leif
Priority: blocker Milestone: sage-5.10
Component: scripts Keywords:
Cc: nthiery, saliola, leif, hivert, jdemeyer Merged in: sage-5.10.rc2
Authors: John Palmieri, Leif Leonhardy Reviewers: John Palmieri, Travis Scrimshaw, Leif Leonhardy
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #14721 Stopgaps:

Description (last modified by leif)

With the attached patches, sage --clone

  • does no longer rebuild the reference manual,
  • does no longer unnecessarily rebuild the whole Sage library (just because of a missing Cython version file).

In e.g. the sage-combinat script, there are usually so many patches applied in the queue, it makes more sense to (re)build the manual after applying the patches, not before.

The unintended rebuild of all extension modules upon cloning is a bug introduced by #13031.


Apply trac_13245-clone-nodocbuild.patch and trac_13245-clone-cython.patch.

Attachments (4)

trac_13245-clone-docbuild.patch (5.8 KB) - added by jhpalmieri 7 years ago.
scripts repo
trac_13245-clone-nodocbuild.patch (2.2 KB) - added by jhpalmieri 7 years ago.
trac_13245-clone-cython.patch (1.7 KB) - added by jhpalmieri 6 years ago.
build_log.txt (450.6 KB) - added by tscrim 6 years ago.

Download all attachments as: .zip

Change History (66)

Changed 7 years ago by jhpalmieri

scripts repo

comment:1 Changed 7 years ago by jhpalmieri

  • Status changed from new to needs_review

Changed 7 years ago by jhpalmieri

comment:2 Changed 7 years ago by jhpalmieri

  • Cc leif hivert added
  • Description modified (diff)

Here is a different patch; this one just turns off docbuilding in sage-clone altogether.

comment:3 follow-up: Changed 6 years ago by leif

Ping.


Not directly related, but meanwhile sage-clone also regenerates all Cython-generated files, because we don't copy .cython_version, which is fixed by:

  • sage-clone

    diff --git a/sage-clone b/sage-clone
    a b  
    5353
    5454cpdir(os.path.abspath('sage/sage'), os.path.abspath(branch + '/sage'))
    5555
     56if os.path.isfile('sage/.cython_version'):
     57    print "Copying over hidden Cython version file..."
     58    os.link('sage/.cython_version', branch+'/.cython_version')
     59
    5660def copy_dtree(src_dir, dest_dir):
    5761    src_root = os.path.abspath(src_dir)
    5862    dest_root = os.path.abspath(dest_dir)


But there are other issues as well (e.g. rebuilding the fast_callable interpreters, which IMHO isn't necessary either).

Last edited 6 years ago by leif (previous) (diff)

comment:4 in reply to: ↑ 3 ; follow-up: Changed 6 years ago by leif

Replying to leif:

But there are other issues as well (e.g. rebuilding the fast_callable interpreters, which IMHO isn't necessary either).

Oh, forgot: Cython-generated header files (*.h) don't contain a special comment on that on the first line (which is probably an upstream Cython bug), so the following currently doesn't work as expected either:

            if ext in ['.h', '.c', '.cpp']:
                if 'Cython' in open(src + '/' + F).readline():
                    os.link(src + '/' + F, dest + '/' +F)
                    os.utime(dest + '/' +F, None)

(For the moment, we could look for ^#ifndef __PYX_HAVE__sage__ in those, probably even without the sage__.)

comment:5 follow-up: Changed 6 years ago by jhpalmieri

  • Description modified (diff)

Okay, here is a patch to deal with the .h files and .cython_version. Rebuilding the fast_callable interpreters doesn't take too much time, so I'm not dealing with it right now. Leif, would you like to be listed as an author?

Changed 6 years ago by jhpalmieri

comment:6 in reply to: ↑ 5 ; follow-up: Changed 6 years ago by leif

Replying to jhpalmieri:

Rebuilding the fast_callable interpreters doesn't take too much time, so I'm not dealing with it right now.

Yes, but it also triggers the rebuild of (currently) at least one other Cython module (besides the newly generated five sage.ext.interpreters.wrapper_*.pyx):

...
Building interpreters for fast_callable
Updating Cython code....
Compiling sage/plot/plot3d/parametric_surface.pyx because it depends on ./sage/ext/interpreters/wrapper_rdf.pxd.
...
building 'sage.plot.plot3d.parametric_surface' extension
...

(I think we could just copy over all files in sage/ext/interpreters/, which is in .hgignore.)

Note sure what or whether doc-rebuilding is triggered (upon next sage --docbuild ...) by rebuilding the interpreters though.


Could you explain why we "touch" almost all manually copied (more precisely, linked) files?


Leif, would you like to be listed as an author?

Does the patch introduce new environment variables? :P


[Haven't tried the patches here yet.]

comment:7 in reply to: ↑ 6 ; follow-up: Changed 6 years ago by leif

Replying to leif:

Could you explain why we "touch" almost all manually copied (more precisely, linked) files?

Ok, seems like Mercurial is to blame. We could use hg clone -U though instead, and create the working copy by ourselves. (Not tested.)

comment:8 in reply to: ↑ 7 ; follow-up: Changed 6 years ago by leif

Replying to leif:

Replying to leif:

Could you explain why we "touch" almost all manually copied (more precisely, linked) files?

Ok, seems like Mercurial is to blame. We could use hg clone -U though instead, and create the working copy by ourselves. (Not tested.)

Or change back the modification times of the files (of the working copy) Mercurial created... ;-)

OMG.

comment:9 in reply to: ↑ 8 Changed 6 years ago by leif

Replying to leif:

Replying to leif:

Ok, seems like Mercurial is to blame. We could use hg clone -U though instead, and create the working copy by ourselves. (Not tested.)

Or change back the modification times of the files (of the working copy) Mercurial created... ;-)

OMG.

Thank you, nanny!

(How about making this optional?!?)

comment:10 Changed 6 years ago by leif

Since apparently hg archive (meanwhile?) doesn't preserve modification times either, we could use hg clone -U and some cp -pPR of hg manifest magic instead (of plain hg clone).

comment:11 follow-up: Changed 6 years ago by jhpalmieri

Given the upcoming transition to git, I don't think it's worth trying to wrestle with Mercurial very much. I don't want to spend much more time on it, anyway.

comment:12 in reply to: ↑ 11 Changed 6 years ago by leif

Replying to jhpalmieri:

Given the upcoming transition to git, I don't think it's worth trying to wrestle with Mercurial very much. I don't want to spend much more time on it, anyway.

Yes, hopefully.

Anyway, the title (and parts of the description) still advertise an option to disable docbuilding, but the currently listed patch just disables it unconditionally. Which way do we want to go?

comment:13 Changed 6 years ago by jhpalmieri

I think it makes more sense not to build the docs at all. But I don't use sage --clone myself, I only opened this in response to experiences of various sage-combinat users.

comment:14 Changed 6 years ago by jhpalmieri

  • Authors changed from John Palmieri to John Palmieri, Leif Leonhardy
  • Reviewers set to John Palmieri
  • Summary changed from sage --clone: give option to not rebuild the docs to sage --clone: do not rebuild the entire Sage library, and do not rebuild the docs

I think that rebuilding the files in sage/ext/interpreters/ is because of code in devel/sage/setup.py, and any changes to this should be on another ticket.

Meanwhile, the patch trac_13245-clone-cython.patch seems to fix the main cloning problem. I'm happy with that one.

comment:15 Changed 6 years ago by leif

  • Priority changed from minor to blocker

Adjusting priority to reflect the changed subject.

Will try to finally review this asap...

comment:16 Changed 6 years ago by leif

  • Type changed from enhancement to defect

comment:17 Changed 6 years ago by leif

  • Description modified (diff)

comment:18 Changed 6 years ago by leif

  • Description modified (diff)

comment:19 in reply to: ↑ 4 ; follow-up: Changed 6 years ago by leif

Replying to leif:

Oh, forgot: Cython-generated header files (*.h) don't contain a special comment on that on the first line (which is probably an upstream Cython bug), so the following currently doesn't work as expected either:

            if ext in ['.h', '.c', '.cpp']:
                if 'Cython' in open(src + '/' + F).readline():
                    os.link(src + '/' + F, dest + '/' +F)
                    os.utime(dest + '/' +F, None)

(For the moment, we could look for ^#ifndef __PYX_HAVE__sage__ in those, probably even without the sage__.)


I'm having another header file in the original but not the clone:

sage/rings/complex_double_api.h, which starts with

#ifndef __PYX_HAVE_API__sage__rings__complex_double
#define __PYX_HAVE_API__sage__rings__complex_double
#include "Python.h"
#include "complex_double.h"

static struct __pyx_obj_4sage_5rings_14complex_double_ComplexDoubleElement *(*__pyx_f_4sage_5rings_14complex_double_new_ComplexDoubleElement)(void) = 0;
#define new_ComplexDoubleElement __pyx_f_4sage_5rings_14complex_double_new_ComplexDoubleElement

#ifndef __PYX_HAVE_RT_ImportModule
#define __PYX_HAVE_RT_ImportModule

(and is explicitly listed in .hgignore).

I couldn't find any places where this is actually used though, nor do I know where it originates from.

comment:20 in reply to: ↑ 19 ; follow-up: Changed 6 years ago by leif

Replying to leif:

I'm having another header file in the original but not the clone:

sage/rings/complex_double_api.h [...] (and is explicitly listed in .hgignore).

I couldn't find any places where this is actually used though, nor do I know where it originates from.

Although it looks Cython-generated, it doesn't get recreated when I touch complex_double* sources.

It seemsTM to be some left-over file we still ship, despite it being in .hgignore; the file date is July 9th 2012...

So it apparently doesn't matter whether we copy / link it or not, but I feel a bit uncomfortable.

comment:21 Changed 6 years ago by jhpalmieri

I think that being in MANIFEST.in is more relevant to what we ship than being in .hgignore, since as far as I can tell, we're using Python's distutils to produce the Sage library spkg (by calling python setup.py sdist in devel/sage/spkg-dist). I'll try deleting the file and running the full test suite...

comment:22 in reply to: ↑ 20 Changed 6 years ago by leif

Replying to leif:

Replying to leif:

I'm having another header file in the original but not the clone:

sage/rings/complex_double_api.h [...] (and is explicitly listed in .hgignore).

I couldn't find any places where this is actually used though, nor do I know where it originates from.

Although it looks Cython-generated, it doesn't get recreated when I touch complex_double* sources.

It seemsTM to be some left-over file we still ship, despite it being in .hgignore; the file date is July 9th 2012...

See #14700 for removing some (shipped but) unused files.

comment:23 follow-ups: Changed 6 years ago by leif

Hmmm, did some testing, and it seems copying over any already built documentation doesn't make sense (or doesn't work as expected at least).

I've made sure that the documentation (and the Sage library) is up-to-date, but if I run sage --docbuild reference html afterwards in a fresh clone, apparently everything (of the reference manual) gets rebuilt.

It says

[foo] building [html]: targets for N source files that are out of date
[foo] updating environment: 0 added, N changed, 0 removed
...

for all components AFAICS.

(This is with 5.10.rc0.)

comment:24 in reply to: ↑ 23 Changed 6 years ago by leif

Replying to leif:

[...] if I run sage --docbuild reference html afterwards in a fresh clone, apparently everything (of the reference manual) gets rebuilt.

It says

[foo] building [html]: targets for N source files that are out of date
[foo] updating environment: 0 added, N changed, 0 removed
...

for all components AFAICS.

The same happens when I switch back to the main branch and (re)run sage --docbuild reference html.

comment:25 in reply to: ↑ 23 ; follow-up: Changed 6 years ago by jhpalmieri

Replying to leif:

Hmmm, did some testing, and it seems copying over any already built documentation doesn't make sense (or doesn't work as expected at least).

I think that it makes sense, but it doesn't work as expected. That is, if you clone the Sage library, you should still be able to find documentation in the usual place. The fact that it needs to be rebuilt is annoying (although less so, now that the docs don't take as long to rebuild), and a bug with Sphinx or mercurial or something we do, I'm not sure which. It's been an issue with cloning for years, and it's never been resolved satisfactorily.

I don't know if cloning is even going to be part of the new git workflow, whenever that arrives. In any case, I don't feel like spending any energy to try to fix this problem.

comment:26 Changed 6 years ago by leif

FWIW,

  • sage-clone

    diff --git a/sage-clone b/sage-clone
    a b  
    9090                    fr = os.path.join(ref, f, directory)
    9191                    if os.path.exists(fr):
    9292                        to = os.path.join(branch, 'doc', lang, 'reference', f, directory)
    93                         cmd = 'cp -pR %s %s' % (fr, to)
     93                        cmd = 'cp -R %s %s' % (fr, to)
    9494                        subprocess.call([cmd], shell=True)
    9595    except:
    9696        pass

(alone, at least) doesn't make a difference.

comment:27 in reply to: ↑ 25 ; follow-ups: Changed 6 years ago by leif

Replying to jhpalmieri:

Replying to leif:

Hmmm, did some testing, and it seems copying over any already built documentation doesn't make sense (or doesn't work as expected at least).

I think that it makes sense, but it doesn't work as expected.

I meant that we should either drop copying over any "pre-built" documentation (since it doesn't work -- there's apparently no difference to building all from scratch), or fix rebuilding [all of it].

Since we both seem to be unable to do (or don't want to spend time on) the latter, I think we should go with the former.


That is, if you clone the Sage library, you should still be able to find documentation in the usual place.

? I don't understand what you mean by that; $SAGE_ROOT/devel/sage-main/doc/output/... at least is always there.


I don't know if cloning is even going to be part of the new git workflow, whenever that arrives.

I don't see why cloning should vanish with the switch to git.


In any case, I don't feel like spending any energy to try to fix this problem.

Me neither.

comment:28 in reply to: ↑ 27 ; follow-up: Changed 6 years ago by jhpalmieri

Replying to leif:

Replying to jhpalmieri:

That is, if you clone the Sage library, you should still be able to find documentation in the usual place.

? I don't understand what you mean by that; $SAGE_ROOT/devel/sage-main/doc/output/... at least is always there.

I want it to be available in $SAGE_ROOT/devel/sage/doc/output/.... If you run tutorial() or reference() from within Sage, it should open up documentation, for example.

Last edited 6 years ago by jhpalmieri (previous) (diff)

comment:29 in reply to: ↑ 27 Changed 6 years ago by jhpalmieri

Replying to leif:

Replying to jhpalmieri:

I don't know if cloning is even going to be part of the new git workflow, whenever that arrives.

I don't see why cloning should vanish with the switch to git.

Okay, but cloning will have to be rewritten to use git instead of mercurial. I don't know what affect that will have on modification times and other issues relating to rebuilding the documentation.

comment:30 Changed 6 years ago by leif

What's most annoying to me is that the whole documentation gets rebuilt (upon sage --docbuild reference html) when I switch back to the original branch (which is supposed to be left untouched).

(I can't believe that's always been the case, at least I don't recall having ever experienced that, but perhaps I simply never tried.)

It seems python setup.py install somehow doesn't play well with Sphinx...

comment:31 in reply to: ↑ 28 Changed 6 years ago by leif

Replying to jhpalmieri:

If you run tutorial() or reference() from within Sage, it should open up documentation, for example.

I did not even know these commands... :-)

Shouldn't they get advertised in the startup banner?

comment:32 Changed 6 years ago by jhpalmieri

The startup banner says Type "help()" for help.. If you type help(), you get information about these commands.

comment:33 follow-up: Changed 6 years ago by tscrim

I tried this with 5.10.rc1 by applying the patches to the script repo ($SAGE_ROOT/local/bin), then running

sage -combinat install

and all of the extensions are being rebuilt for me. Did I need to run sage -b first? (I did like the fact that I didn't have to wait for the doc to be rebuilt.)

comment:34 follow-up: Changed 6 years ago by tscrim

I also just ran

sage -b main

and this is recompiling all of the extension classes as well.

travis@travis-virtualbox:~/sage-5.10.rc1/devel/sage-main$ sage -b main

----------------------------------------------------------
sage: Building and installing modified Sage library files.


Installing c_lib
scons: `install' is up to date.
Updating Cython code....
Finished compiling Cython code (time = 33.1963000298 seconds)
running install
running build
running build_py
running build_ext
building 'sage.algebras.quatalg.quaternion_algebra_element' extension
building 'sage.algebras.letterplace.free_algebra_letterplace' extension
building 'sage.algebras.letterplace.free_algebra_element_letterplace' extension
building 'sage.algebras.letterplace.letterplace_ideal' extension
building 'sage.algebras.quatalg.quaternion_algebra_cython' extension
building 'sage.calculus.var' extension
building 'sage.calculus.riemann' extension
building 'sage.calculus.interpolators' extension
building 'sage.categories.action' extension
building 'sage.categories.category_singleton' extension
building 'sage.categories.functor' extension
building 'sage.categories.map' extension
building 'sage.categories.morphism' extension
building 'sage.categories.examples.semigroups_cython' extension
building 'sage.coding.binary_code' extension
building 'sage.combinat.expnums' extension
...

Could this be a result of the changes to sync-build? (I'd be surprised if it was since this what I got when doing the -combinat install.)

Last edited 6 years ago by tscrim (previous) (diff)

comment:35 in reply to: ↑ 33 Changed 6 years ago by leif

Replying to tscrim:

I tried this with 5.10.rc1 by applying the patches to the script repo ($SAGE_ROOT/local/bin), then running

sage -combinat install

and all of the extensions are being rebuilt for me. Did I need to run sage -b first?

a) If you had committed changes to the main repo, you should of course run sage -b before cloning. (It would be better to test with a vanilla Sage library.)

b) Are you sure everything (= all extension modules) got rebuilt, as opposed to just changed modules and those that depend on the changed ones (the latter potentially being many, even if not many got changed)?

c) Is that a regression?

(The current brokenness of sage -sync-build might be related, although I don't see that sage -combinat calls it.)

comment:36 in reply to: ↑ 34 Changed 6 years ago by leif

Replying to tscrim:

I also just ran

sage -b main

and this is recompiling all of the extension classes as well.

travis@travis-virtualbox:~/sage-5.10.rc1/devel/sage-main$ sage -b main

----------------------------------------------------------
sage: Building and installing modified Sage library files.


Installing c_lib
scons: `install' is up to date.
Updating Cython code....
Finished compiling Cython code (time = 33.1963000298 seconds)
running install
running build
running build_py
running build_ext
building 'sage.algebras.quatalg.quaternion_algebra_element' extension
building 'sage.algebras.letterplace.free_algebra_letterplace' extension
building 'sage.algebras.letterplace.free_algebra_element_letterplace' extension
building 'sage.algebras.letterplace.letterplace_ideal' extension
building 'sage.algebras.quatalg.quaternion_algebra_cython' extension
building 'sage.calculus.var' extension
building 'sage.calculus.riemann' extension
building 'sage.calculus.interpolators' extension
building 'sage.categories.action' extension
building 'sage.categories.category_singleton' extension
building 'sage.categories.functor' extension
building 'sage.categories.map' extension
building 'sage.categories.morphism' extension
building 'sage.categories.examples.semigroups_cython' extension
building 'sage.coding.binary_code' extension
building 'sage.combinat.expnums' extension
...

The messages don't seem to be in chronological order (probably stdout and stderr out of sync); I somehow doubt all extension modules got rebuilt if "re-cythonizing" took just 33 seconds...


Could this be a result of the changes to sync-build?

There are no changes to sage -sync-build yet.

comment:37 Changed 6 years ago by leif

P.S.:

sage --clone foo; sage -b main works for me (modulo that the fast_callable interpreters [and IIRC six extension modules that depend on them] get rebuilt in both cases, which is a known but in our opinion minor issue we didn't want to address here).

comment:38 Changed 6 years ago by leif

Travis, the interesting lines should be

Executing N commands (using M threads)
...
Time to execute N commands: n seconds
...
Total time spent compiling C/C++ extensions: m seconds.

comment:39 Changed 6 years ago by leif

From a clean sage-main repo,

$ (./sage --clone foo; ./sage -b main) 2>&1 | egrep '^(building|Cythonizing|Compiling)'

gives me

Compiling sage/plot/plot3d/parametric_surface.pyx because it depends on ./sage/ext/interpreters/wrapper_rdf.pxd.
Compiling sage/ext/interpreters/wrapper_rdf.pyx because it changed.
Compiling sage/ext/interpreters/wrapper_cdf.pyx because it changed.
Compiling sage/ext/interpreters/wrapper_rr.pyx because it changed.
Compiling sage/ext/interpreters/wrapper_py.pyx because it changed.
Compiling sage/ext/interpreters/wrapper_el.pyx because it changed.
building 'sage.plot.plot3d.parametric_surface' extension
building 'sage.ext.interpreters.wrapper_rdf' extension
building 'sage.ext.interpreters.wrapper_cdf' extension
building 'sage.ext.interpreters.wrapper_rr' extension
building 'sage.ext.interpreters.wrapper_py' extension
building 'sage.ext.interpreters.wrapper_el' extension
Cythonizing sage/ext/interpreters/wrapper_py.pyx
Cythonizing sage/ext/interpreters/wrapper_rdf.pyx
Cythonizing sage/plot/plot3d/parametric_surface.pyx
Cythonizing sage/ext/interpreters/wrapper_el.pyx
Cythonizing sage/ext/interpreters/wrapper_rr.pyx
Cythonizing sage/ext/interpreters/wrapper_cdf.pyx

comment:40 follow-up: Changed 6 years ago by tscrim

a) I only applied the two patches to the scripts repo, nothing to the main repo (I had just built sage, didn't even start it yet).

b) Not 100% but almost since there are far fewer extensions that get rebuilt after pushing/popping the entire combinat queue, and it seemed like the same number of modules after running -sync-build (which currently erases all cython files). I can't tell you the interesting lines because I didn't think I would need it (and I really don't want to go through rebuilding it all again) At least afterwards, switching between branches is fast. Nevertheless I would think changes in one branch's built extensions would not affect another's...

Did you run

sage --clone foo
sage -b foo

and then

sage -b main

?

comment:41 in reply to: ↑ 40 Changed 6 years ago by leif

Replying to tscrim:

Did you run

sage --clone foo
sage -b foo

and then

sage -b main

?

Not explicitly, since sage-clone already runs sage -b in the new clone.

comment:42 Changed 6 years ago by jhpalmieri

Just for reference: with Sage 5.10.rc1 (not that the version should matter, as far as I know), I applied the two patches here and did

./sage -b   # to make sure everything was up to date
./sage --combinat install

and it only built 6 Cython files before applying the combinat queue:

Building interpreters for fast_callable
Updating Cython code....
Compiling sage/plot/plot3d/parametric_surface.pyx because it depends on ./sage/ext/interpreters/wrapper_rdf.pxd.
Compiling sage/ext/interpreters/wrapper_rdf.pyx because it changed.
Compiling sage/ext/interpreters/wrapper_cdf.pyx because it changed.
Compiling sage/ext/interpreters/wrapper_rr.pyx because it changed.
Compiling sage/ext/interpreters/wrapper_py.pyx because it changed.
Compiling sage/ext/interpreters/wrapper_el.pyx because it changed.
Cythonizing sage/ext/interpreters/wrapper_cdf.pyx
Cythonizing sage/ext/interpreters/wrapper_el.pyx
Cythonizing sage/ext/interpreters/wrapper_py.pyx
Cythonizing sage/ext/interpreters/wrapper_rdf.pyx
Cythonizing sage/ext/interpreters/wrapper_rr.pyx
Cythonizing sage/plot/plot3d/parametric_surface.pyx
Finished compiling Cython code (time = 17.591463089 seconds)
running install
running build
running build_py
running build_ext
warning: Replacing library search directory in linker command:
  "/Users/palmieri/Desktop/Sage_stuff/sage_builds/clean/sage-5.10.rc1/local/lib" -> "/Users/palmieri/Desktop/Sage_stuff/sage_builds/sage-5.10.rc1/local/lib"

building 'sage.plot.plot3d.parametric_surface' extension
building 'sage.ext.interpreters.wrapper_rdf' extension
building 'sage.ext.interpreters.wrapper_cdf' extension
building 'sage.ext.interpreters.wrapper_rr' extension
building 'sage.ext.interpreters.wrapper_py' extension
building 'sage.ext.interpreters.wrapper_el' extension
Executing 6 commands (using 2 threads)
...

comment:43 Changed 6 years ago by tscrim

Here's a snippet of what I get -- with no patches applied, double-checking the patches are applied in local/bin, and then running sage -b -- when I run

sage --clone foo
...
Building interpreters for fast_callable
Updating Cython code....
Compiling sage/plot/plot3d/parametric_surface.pyx because it depends on ./sage/ext/interpreters/wrapper_rdf.pxd.
Compiling sage/ext/interpreters/wrapper_rdf.pyx because it changed.
Compiling sage/ext/interpreters/wrapper_cdf.pyx because it changed.
Compiling sage/ext/interpreters/wrapper_rr.pyx because it changed.
Compiling sage/ext/interpreters/wrapper_py.pyx because it changed.
Compiling sage/ext/interpreters/wrapper_el.pyx because it changed.
Cythonizing sage/ext/interpreters/wrapper_cdf.pyx
Cythonizing sage/ext/interpreters/wrapper_el.pyx
Cythonizing sage/ext/interpreters/wrapper_py.pyx
Cythonizing sage/ext/interpreters/wrapper_rdf.pyx
Cythonizing sage/ext/interpreters/wrapper_rr.pyx
Cythonizing sage/plot/plot3d/parametric_surface.pyx
Finished compiling Cython code (time = 70.2989528179 seconds)
running install
running build
running build_py
running build_ext
building 'sage.algebras.quatalg.quaternion_algebra_element' extension
building 'sage.algebras.letterplace.free_algebra_letterplace' extension
...
building 'sage.ext.interpreters.wrapper_py' extension
building 'sage.ext.interpreters.wrapper_el' extension
Executing 343 commands (using 1 thread)

I'll upload a full log once it's done. Could it be because I didn't run sage -b after first installing the patches the first time around? Either that or (more likely) it's something specific to me or my system...

Changed 6 years ago by tscrim

comment:44 Changed 6 years ago by tscrim

Here's a log from calling sage -b main afterwards (because I stupidly overwrote the first log).

comment:45 Changed 6 years ago by jhpalmieri

When I called sage -b main afterwards, I got

----------------------------------------------------------
sage: Building and installing modified Sage library files.


Installing c_lib
scons: `install' is up to date.
Updating Cython code....
Finished compiling Cython code (time = 11.5694179535 seconds)
running install
running build
running build_py
running build_ext
warning: Replacing library search directory in linker command:
  "/Users/palmieri/Desktop/Sage_stuff/sage_builds/clean/sage-5.10.rc1/local/lib" -> "/Users/palmieri/Desktop/Sage_stuff/sage_builds/sage-5.10.rc1/local/lib"

Executing 0 commands (using 1 thread)
Time to execute 0 commands: 0.0257179737091 seconds
Total time spent compiling C/C++ extensions:  0.278106927872 seconds.
running install_lib
running install_egg_info
Removing /Users/palmieri/Desktop/Sage_stuff/sage_builds/sage-5.10.rc1/local/lib/python2.7/site-packages/sage-0.0.0-py2.7.egg-info
Writing /Users/palmieri/Desktop/Sage_stuff/sage_builds/sage-5.10.rc1/local/lib/python2.7/site-packages/sage-0.0.0-py2.7.egg-info

real	0m17.365s
user	0m6.021s
sys	0m2.604s

So it looks like there is something odd with your system.

comment:46 Changed 6 years ago by leif

Replying to leif:

Replying to tscrim:

Did you run

sage --clone foo
sage -b foo

and then

sage -b main

?

Not explicitly, since sage-clone already runs sage -b in the new clone.

But even when I do, that doesn't make any difference, as expected.

comment:47 Changed 6 years ago by leif

P.S.: I also touched some pyx files in the clone, ran sage -b, then sage -b main. Not surprisingly, the latter didn't trigger any rebuilds either.

comment:48 Changed 6 years ago by jhpalmieri

Just to make sure, when you run ./sage --hg -R local/bin log, it shows that the two patches have been applied, right?

comment:49 Changed 6 years ago by tscrim

travis@travis-virtualbox:~/sage-5.10.rc1$ sage --hg -R local/bin log | head -n 21
changeset:   1953:64e1b84c2d39
tag:         qtip
tag:         tip
tag:         trac_13245-clone-nodocbuild.patch
user:        J. H. Palmieri <palmieri@math.washington.edu>
date:        Mon Mar 11 10:53:04 2013 -0700
summary:     Don't rebuild docs after cloning.

changeset:   1952:148e4ed48517
tag:         qbase
tag:         trac_13245-clone-cython.patch
user:        J. H. Palmieri <palmieri@math.washington.edu>
date:        Mon Mar 11 10:53:04 2013 -0700
summary:     sage-clone: copy over autogenerated .h files, and also .cython_version

changeset:   1951:0eb3c4df6d10
tag:         qparent
user:        Jeroen Demeyer <jdemeyer@cage.ugent.be>
date:        Wed Jun 05 22:11:28 2013 +0000
summary:     5.10.rc1

So it's probably just something I messed up with my install (my guess by applying the patches and then running -combinat install before (rebuilding and) starting sage) and/or system.

comment:50 follow-up: Changed 6 years ago by leif

I thinkTM I know what the problem might be, namely the order in which subdirectories of devel/sage/build/ are copied.

Since the odd addition of build_dir in setup.py, which already breaks sage-sync-build, the Cython-generated .c and .cpp files also end up below build/.

Now if these get copied (and touched!) after their corresponding .o and .so files, the latter will get rebuilt.


Travis, could you switch back to the main branch, remove the line setting build_dir from setup.py (or change build_dir='build/cythonized' to build_dir=None), do sage -b to get a clean new main repo, and retry your test? B)

comment:51 Changed 6 years ago by tscrim

Doing so right now, although the change seems to have triggered a recompile/recythonizing of the cython files (it might have been a side effect of one of my other tests). However I need to sleep, so I'll finish the test in the morning. Thanks.

comment:52 in reply to: ↑ 50 Changed 6 years ago by leif

Replying to leif:

I thinkTM I know what the problem might be, namely the order in which subdirectories of devel/sage/build/ are copied.

Since the odd addition of build_dir in setup.py, which already breaks sage-sync-build, the Cython-generated .c and .cpp files also end up below build/.

Now if these get copied (and touched!) after their corresponding .o and .so files, the latter will get rebuilt.

With

def copy_dtree(src_dir, dest_dir):
    src_root = os.path.abspath(src_dir)
    dest_root = os.path.abspath(dest_dir)

    for root, dirs, files in os.walk(src_root):
        nroot = dest_root + root[len(src_root):]
        for d in dirs:
            os.makedirs(nroot+'/'+d)
        for f in files:
            if os.path.splitext(f)[1] in [".c",".cpp",".o",".so"]: # ADDED
                print("  %s" % f) # ADDED
            os.link(root+'/'+f,nroot+'/'+f)
            os.utime(nroot+'/'+f, None)

sys.stdout.flush(); sys.stderr.flush() # ADDED
print "Copying over build directory..."
copy_dtree('sage/build', branch + '/build')
sys.stdout.flush(); sys.stderr.flush() # ADDED

I can at least confirm that for me all .o and .so files get copied after all .c and .cpp files have been copied -- rather incidentally I think.

comment:53 Changed 6 years ago by tscrim

My .o files get copied, then my .c and .cpp, then my .so (with the changes to setup.py).

comment:54 Changed 6 years ago by tscrim

Also I only get the rebuilding of the 7 extensions and switch back to main is trivial.

comment:55 follow-ups: Changed 6 years ago by jdemeyer

  • Cc jdemeyer added

Any progress? Does this still deserve to be a Sage 5.10 blocker?

comment:56 in reply to: ↑ 55 ; follow-up: Changed 6 years ago by leif

Replying to jdemeyer:

Any progress? Does this still deserve to be a Sage 5.10 blocker?

At least the .cython_version part here.

And we should really disable (postpone) Cython-generated files "out-of-tree", since this breaks both sage-clone and sage-sync-build, i.e., set build_dir to None in setup.py.

([Even] with that, upgrading from beta4+ will still trigger the rebuild of the whole Sage library, but only once.)

comment:57 in reply to: ↑ 56 Changed 6 years ago by leif

Replying to leif:

And we should really disable (postpone) Cython-generated files "out-of-tree", since this breaks both sage-clone and sage-sync-build, i.e., set build_dir to None in setup.py.

Inline patch, worth its own (blocker) ticket:

  • setup.py

    diff --git a/setup.py b/setup.py
    a b  
    537537    ext_modules = cythonize(
    538538        ext_modules,
    539539        nthreads = int(os.environ.get('SAGE_NUM_THREADS', 0)),
    540         build_dir = 'build/cythonized',
     540        build_dir = None, # Don't "cythonize out-of-tree" (cf. #14570) until
     541                          # sage-clone and sage-sync-build can deal with that.
    541542        force=force)
    542543
    543544    open(version_file, 'w').write(Cython.__version__)

EDIT: Added ticket reference to the patch.

Last edited 6 years ago by leif (previous) (diff)

comment:58 in reply to: ↑ 55 ; follow-up: Changed 6 years ago by leif

Replying to jdemeyer:

Any progress?

I was (more or less reluctantly) going to give this positive review, but then Travis discovered that using build_dir can also break sage-clone.

So I'm happy provided we make this ticket depend on the above patch / its ticket.

comment:59 in reply to: ↑ 55 Changed 6 years ago by nthiery

Replying to jdemeyer:

Any progress? Does this still deserve to be a Sage 5.10 blocker?

It would be great indeed to have 5.10 ready for the Sage-Combinat days next week! There is a lot of good stuff there.

comment:60 in reply to: ↑ 58 ; follow-up: Changed 6 years ago by leif

  • Dependencies set to #14721
  • Reviewers changed from John Palmieri to John Palmieri, Travis Scrimshaw, Leif Leonhardy
  • Status changed from needs_review to positive_review

Replying to leif:

Replying to jdemeyer:

Any progress?

I was (more or less reluctantly) going to give this positive review, but then Travis discovered that using build_dir can also break sage-clone.

So I'm happy provided we make this ticket depend on the above patch / its ticket.

I've now created #14721.

comment:61 in reply to: ↑ 60 Changed 6 years ago by leif

Replying to leif:

Replying to leif:

So I'm happy provided we make this ticket depend on the above patch / its ticket.

I've now created #14721.

Needs review... :P

comment:62 Changed 6 years ago by jdemeyer

  • Merged in set to sage-5.10.rc2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.