Opened 12 years ago

Closed 12 years ago

#4500 closed defect (fixed)

[with patch, positive review] cython files missing from build directory after install

Reported by: craigcitro Owned by: craigcitro
Priority: blocker Milestone: sage-3.2
Component: build Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

Immediately after building sage from source, the cython files from the sage library aren't in $SAGE_ROOT/devel/sage/build/sage where they belong. One can see them get copied when doing a sage -b or sage -clone, but why aren't they there in the first place?

Even stranger -- they get copied over during the build, as one can see in install.log ... where do they get deleted?

Attachments (3)

4500_sage-build.patch (1.6 KB) - added by GeorgSWeber 12 years ago.
now tested
4500_sage-build-patch2.patch (678 bytes) - added by GeorgSWeber 12 years ago.
apply after the first one
trac-4500.patch (1.3 KB) - added by craigcitro 12 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 12 years ago by mabshoff

  • Milestone changed from sage-3.2.1 to sage-3.2
  • Priority changed from major to blocker

If there ever was a blocker for 3.2 this would be one :)

Cheers,

Michael

comment:2 Changed 12 years ago by GeorgSWeber

Unbelievable! But true (I checked older Sage versions, too). I read the ticket while building Sage 3.2.rc0 and had a look. While in the shell where I issued the build it was printing:

g++ -o libcsage.dylib -single_module -flat_namespace -undefined dynamic_lookup -dynamiclib src/convert.os src/interrupt.os src/mpn_pylong.os src/mpz_pylong.os src/stdsage.os src/gmp_globals.os src/ZZ_pylong.os src/ntl_wrap.os -L/Users/georgweber/Public/sage/sage-3.2.rc0/local/lib -lntl -lgmp -lpari
*** TOUCHING ALL CYTHON (.pyx) FILES ***
scons: `install' is up to date.

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


Installing c_lib
scons: `install' is up to date.
Updating Cython code....
sage/structure/sage_object.pyx --> /Users/georgweber/Public/sage/sage-3.2.rc0/local//lib/python/site-packages//sage/structure/sage_object.pyx
sage/structure/category_object.pyx --> /Users/georgweber/Public/sage/sage-3.2.rc0/local//lib/python/site-packages//sage/structure/category_object.pyx

one should assume that then in the following the .pyx files were copied over.

But this was not the case!

In fact, the directory /Users/georgweber/Public/sage/sage-3.2.rc0/devel/sage/build/sage/ was empty (!!!) at that time, and thus the (linked) directory /Users/georgweber/Public/sage/sage-3.2.rc0/locallib/python/site-packages/sage/ was empty too, i.e. had no subdirectory structure. I wasn't fast enough to create by hand some of the missung directories and see whether then, the .pyx files would be copied over there as the log output shows resp. wants to make us believe, but that might be worth another try.

I do now think the .pyx were not deleted, but were never successfully copied over.

comment:3 Changed 12 years ago by GeorgSWeber

First patch doesn't work of course (I forgot one subdirectory level)

Changed 12 years ago by GeorgSWeber

now tested

comment:4 Changed 12 years ago by GeorgSWeber

  • Owner changed from mabshoff to GeorgSWeber
  • Status changed from new to assigned
  • Summary changed from cython files missing from build directory after install to [with patch, needs review] cython files missing from build directory after install

Ahh, I didn't forget a subdirectory level, but forgot to take into account that the directory "build" under "devel/sage" does not exist at that time.

Works fine at my install, please review.

comment:5 Changed 12 years ago by GeorgSWeber

  • Summary changed from [with patch, needs review] cython files missing from build directory after install to [with patch, needs work] cython files missing from build directory after install

Zut alors, it seems I have to recheck this one ... still working

comment:6 Changed 12 years ago by GeorgSWeber

Before the current patch:

Time to execute 216 commands: 2665.62391996 seconds
Finished compiling Cython code (time = 2668.81615806 seconds)

after the current patch:

Time to execute 95 commands: 967.580175877 seconds
Finished compiling Cython code (time = 969.730924129 seconds)

And then things go awry because more than half of the needed Cython compiled files are missing ...

Changed 12 years ago by GeorgSWeber

apply after the first one

comment:7 Changed 12 years ago by GeorgSWeber

  • Summary changed from [with patch, needs work] cython files missing from build directory after install to [with patch, needs review] cython files missing from build directory after install

Yeah, the patch2 for the patch is coyote ugly, but does the job.

And probably we don't need anymore to "touch all Cython files" in the next lines after the added ones, but for the time being I let that stand as it is.

This ticket (both patches are needed) is for review now (again).

Changed 12 years ago by craigcitro

comment:8 Changed 12 years ago by GeorgSWeber

  • Owner changed from GeorgSWeber to craigcitro
  • Status changed from assigned to new

Craigs approach is way more elegant than mine. I hope his patch passes the necessary (and timeconsuming) tests. I never imagined I could ever write code this ugly as I did, and would be happy if it wasn't necessary to include it into Sage ;-)

Cheers, gsw

comment:9 Changed 12 years ago by craigcitro

  • Status changed from new to assigned

Well, you pointing out what going on made my job much easier! :)

So does that translate to "positive review"?

comment:10 follow-up: Changed 12 years ago by GeorgSWeber

This will lead to a positive review in the end, I'm sure.

But from past experience, I would like to see with my own eyes the patch passing the necessary tests before I give the "go!". It will take at least several hours, maybe a day, before I can do the testing, because right now I don't have access to a Sage installation.

Maybe someone else is quicker.

Cheers, gsw

comment:11 in reply to: ↑ 10 Changed 12 years ago by mabshoff

Replying to GeorgSWeber:

This will lead to a positive review in the end, I'm sure.

But from past experience, I would like to see with my own eyes the patch passing the necessary tests before I give the "go!". It will take at least several hours, maybe a day, before I can do the testing, because right now I don't have access to a Sage installation.

Yeah, any patch to the build system gets additional scrutiny since screw ups here affect a lot of people.

Maybe someone else is quicker.

I will do a full cycle, i.e. force a complete rebuild after devel/sage-main with the patch applied to the spkg to see what happens, i.e. if a "sage -b" forces a rebuild on all files.

Either way: thanks to both of you of putting this issue down.

Cheers, gsw

Cheers,

Michael

comment:12 Changed 12 years ago by mabshoff

Hmm, this blows up (also with the patch from #4480 applied) when doing a virgin build:

Installing c_lib
scons: `install' is up to date.
Updating Cython code....
Traceback (most recent call last):
  File "setup.py", line 435, in <module>
    queue = compile_command_list(ext_modules, deps)
  File "setup.py", line 400, in compile_command_list
    dest_time = deps.timestamp(dest_file)
  File "setup.py", line 244, in timestamp
    self._timestamps[filename] = os.path.getmtime(filename)
  File "/scratch/mabshoff/release-cycle/sage-3.1.3.final/local/lib/python2.5/posixpath.py", line 143, in getmtime
    return os.stat(filename).st_mtime
OSError: [Errno 2] No such file or directory: '/scratch/mabshoff/release-cycle/sage-3.1.3.final/local//lib/python/site-packages//sage/structure/sage_object.pyx'
sage: There was an error installing modified sage library code.

Don't let the path names fool you - this is a 3.2.rc0 build.

Cheers,

Michael

comment:13 Changed 12 years ago by mabshoff

This patch fixes the issue for me:

diff -r c543000d6447 setup.py
--- a/setup.py	Thu Nov 13 05:32:07 2008 -0800
+++ b/setup.py	Thu Nov 13 09:43:33 2008 -0800
@@ -241,7 +241,10 @@
         Look up the last modified time of a file, with caching. 
         """
         if filename not in self._timestamps:
-            self._timestamps[filename] = os.path.getmtime(filename)
+            try:
+                 self._timestamps[filename] = os.path.getmtime(filename)
+            except:
+                 self._timestamps[filename] = 0
         return self._timestamps[filename]
 
     def parse_deps(self, filename, verify=True):

I would guess this is more a #4480 issue, but since I started on this ticket and I want to merge both of them I will mention it here.

Cheers,

Michael

comment:14 Changed 12 years ago by craigcitro

I give a positive review to mabshoff's suggested fix ... now we just need someone to review this patch. :)

comment:15 Changed 12 years ago by robertwb

  • Summary changed from [with patch, needs review] cython files missing from build directory after install to [with patch, positive review] cython files missing from build directory after install

comment:16 Changed 12 years ago by mabshoff

  • Resolution set to fixed
  • Status changed from assigned to closed

Merged trac-4500.patch in Sage 3.2.rc1

Note: See TracTickets for help on using tickets.