Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#11906 closed defect (duplicate)

PolyBoRi 0.7.1 should obey some standard environment variables

Reported by: AlexanderDreyer Owned by: AlexanderDreyer
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: algebra Keywords:
Cc: leif Merged in:
Authors: Reviewers: Jeroen Demeyer, Alexander Dreyer
Report Upstream: Fixed upstream, in a later stable release. Work issues:
Branch: Commit:
Dependencies: #11756 Stopgaps:

Description (last modified by AlexanderDreyer)

It was discovered in #11575 that PolyBoRi's build system does not export all relevant environment variables to the build environment.

The PolyBoRi spkg already treats the following: CC CXX SAGESOFLAGS PYTHONHOME SAGE_LOCAL

Yet, the following standard environment variables are not utilized in PolyBoRi's build process: CFLAGS CXXFLAGS CPPFLAGS LDFLAGS

The following environment variables should be exported to PolyBoRi's build environment: LD_LIBRARY_PATH LIBRARY_PATH LD_RUN_PATH DYLD_RUN_PATH

In addition the previous installation is only deleted after a successful build.

Current patches/spkg

Change History (23)

comment:1 Changed 8 years ago by AlexanderDreyer

  • Owner changed from AlexGhitza to AlexanderDreyer

comment:2 Changed 8 years ago by AlexanderDreyer

There are two kind of environment variables. First, there are PATH, LD_LIBRARY_PATH, CPATH (used by gcc} and similar ones. Those just have to be imported to the build environment, for instance by the following patch to PolyBoRi's SConstruct file:

@@ -377,17 +377,12 @@                                                                                                           
                                                                                                                                
 tools +=  ["disttar", "doxygen"]                                                                                               
                                                                                                                                
-# Get paths an related things from current environment                                                                         
-# note: we cannot avoid those due to non-standard system setups                                                                
-getenv = dict()                                                                                                                
-for key in ['PATH', 'HOME', 'LD_LIBRARY_PATH'] :                                                                               
-    try:                                                                                                                       
-        getenv[key] = os.environ[key]                                                                                          
-    except KeyError:                                                                                                           
-        pass                                                                                                                   
+# Get paths and related things from current environment os.environ                                                             
+# note: We cannot avoid those due to non-standard system setups,                                                               
+#       also we do not know which variables are used in general                                                                
                                                                                                                                
-                                                                                                                               
-env = Environment(ENV = getenv, options = opts, tools = tools, toolpath = '.')                                                 
+env = Environment(ENV = os.environ, options = opts, tools = tools,                                                             
+                  toolpath = '.')                                                                                              
                                                                                                                                
 env['RPATH'] = env.Literal('\\$$ORIGIN/') 

On the other hand, there are variables like CFLAGS, LDFLAGS... Those are not used by the compiler directly, but by make-based build systems. So we have to tell scons what we intended with them. For instance, we have to add a line like the following to the configuration file custom.py in the spkg:

CXXFLAGS=os.environ['CXXFLAGS']

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

It turned out that the build (and partially "Configure", by not finding the GD and M4RI libraries) was broken for me just because LIBRARY_PATH was missing in the environment set up by (or for) SConstruct.

So using

env = Environment(ENV = os.environ, options = opts, tools = tools,                                                             
                  toolpath = '.')

(in SConstruct) would work for me, as well as just adding LIBRARY_PATH to the "exported" environment variables.

Another option is to add all the directories from LIBRARY_PATH to LIBPATH (in custom.py):

if os.environ.has_key("LIBRARY_PATH"): 
    LIBPATH += os.environ["LIBRARY_PATH"].split(os.pathsep)

(Cf. this comment at #11575.)


It's in general of course not bad to export (and/or make use of) other standard environment variables, like CFLAGS etc., too, as mentioned in the ticket's description.

comment:4 in reply to: ↑ 3 Changed 8 years ago by AlexanderDreyer

  • Status changed from new to needs_info

Replying to leif:

It's in general of course not bad to export (and/or make use of) other standard environment variables, like CFLAGS etc., too, as mentioned in the ticket's description.

Yeah, this will be default now in 0.8.1. We change here one crucial paradigm (better explicit than implict variable import). But since one cannot know all set ups, this is the best was to do.

I'll prepare new spkgs soon.

comment:5 Changed 8 years ago by leif

SPKG.txt should have a Special Update/Build? Instructions section (e.g. mentioning that some upstream parts should get deleted, cf. #9472).

Also, spkg/standard/deps have been fixed long ago to make PolyBoRi also depend on GD; cf. #9712. (I.e., the FIXME note in SPKG.txt should also be removed.)

comment:6 Changed 8 years ago by AlexanderDreyer

Here the updates spkg for the 0.7 series. http://boxen.math.washington.edu/home/dreyer/spkg/polybori-0.7.1.p7.spkg PolyBoRi 0.8 will follow soon.

comment:7 Changed 8 years ago by AlexanderDreyer

  • Dependencies changed from #11575 to #11756
  • Status changed from needs_info to needs_review
  • Summary changed from PolyBoRi should obey some standard environment variables to PolyBoRi 0.7.1 should obey some standard environment variables

Delivering a fix for polybori 0.7.1 this ticket should depend on #11756. I'll open another ticket for 0.8.

comment:8 Changed 8 years ago by AlexanderDreyer

  • Authors set to AlexanderDreyer
  • Report Upstream changed from Not yet reported upstream; Will do shortly. to Fixed upstream, in a later stable release.

comment:9 Changed 8 years ago by AlexanderDreyer

  • Description modified (diff)

comment:10 Changed 8 years ago by AlexanderDreyer

  • Description modified (diff)

Since I had to backport another patch from 0.8 (prepend local paths), I rebundled another spkg (at the same place above).

comment:11 Changed 8 years ago by AlexanderDreyer

Note the corresponding ticket for 0.8 is #11909.

comment:12 follow-up: Changed 8 years ago by AlexanderDreyer

The new spkg builds, installs and passes all tests on SuSE 11 amd64 and OS X 10.5 ppc.

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

Replying to AlexanderDreyer:

The new spkg builds, installs and passes all tests on SuSE 11 [...]

In that case, you could also give the new readline 6.2 spkg at #11882 a try... :P

(We couldn't yet test it on OpenSUSE, i.e. nobody had access to an OpenSUSE system; I'm pretty sure the work-around is meanwhile obsolete such that we should remove or disable it. I've only tested it on SLES 11.1 where the work-around -- more or less incidentally -- isn't applied.)

comment:14 in reply to: ↑ 13 Changed 8 years ago by AlexanderDreyer

  • Owner changed from AlexanderDreyer to (none)

Replying to leif:

In that case, you could also give the new readline 6.2 spkg at #11882 a try... :P

If works fine!

(We couldn't yet test it on OpenSUSE, i.e. nobody had access to an OpenSUSE system; I'm pretty sure the work-around is meanwhile obsolete such that we should remove or disable it. I've only tested it on SLES 11.1 where the work-around -- more or less incidentally -- isn't applied.)

OpenSuSE 11.1 and SLES 11 are binary compatible. But it makes a difference whether SP1 was applied to one of them. (I would recomment to apply the service pack anyway.)

comment:15 Changed 8 years ago by AlexanderDreyer

  • Owner changed from (none) to AlexanderDreyer

comment:16 Changed 8 years ago by jdemeyer

  • Milestone sage-4.7.3 deleted

Milestone sage-4.7.3 deleted

comment:17 follow-up: Changed 8 years ago by jdemeyer

  • Milestone set to sage-5.0
  • Status changed from needs_review to needs_info

How does this relate to #12750 and #12655?

comment:18 in reply to: ↑ 17 Changed 8 years ago by AlexanderDreyer

Replying to jdemeyer:

How does this relate to #12750 and #12655?

The fix was merge upstream into the 0.8.1 branch, so #12655 includes it. #12750 only partially (CXXFLAGS etc.).

comment:19 follow-up: Changed 8 years ago by AlexanderDreyer

I'd suggest that I provide a new polybori.0.8.0.p3.spkg with the environment patch in #11909, but not for #12655 (0.8.1).

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

Replying to AlexanderDreyer:

I'd suggest that I provide a new polybori.0.8.0.p3.spkg with the environment patch in #11909, but not for #12655 (0.8.1).

If #12655 already includes the patch, I don't see the need to do this.

Why not close this ticket as duplicate of #12655?

comment:21 in reply to: ↑ 20 Changed 8 years ago by AlexanderDreyer

Replying to jdemeyer:

Why not close this ticket as duplicate of #12655?

I'm fine with that. (In fact, I'd prefer resolving as duplicate, since that issue is really obsolete now.)

comment:22 Changed 8 years ago by jdemeyer

  • Authors AlexanderDreyer deleted
  • Milestone changed from sage-5.0 to sage-duplicate/invalid/wontfix
  • Resolution set to duplicate
  • Reviewers set to Jeroen Demeyer, Alexander Dreyer
  • Status changed from needs_info to closed

comment:23 Changed 8 years ago by leif

Well, this ticket isn't really a duplicate of #12655, it's just (more or less incidentally) superseded by #12655, since upstream also fixed this in the later release.

So until we've upgraded PolyBoRi to 0.8.1 (for other reasons as well), this is still an open issue, although not with PolyBoRi 0.7.1, but 0.8.0, our current spkg. #11909 addresses this... :-)

Note: See TracTickets for help on using tickets.