Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add auto-rebasing mechanism for Cygwin #15423

Closed
jpflori opened this issue Nov 15, 2013 · 23 comments
Closed

Add auto-rebasing mechanism for Cygwin #15423

jpflori opened this issue Nov 15, 2013 · 23 comments

Comments

@jpflori
Copy link

jpflori commented Nov 15, 2013

As explained in the old description, it is often necessary when building Sage on Cygwin to rebase its DLLs. This is the process of updating the default base addresses in memory at which a list of DLLs are loaded, such that none of those DLLs overlap each other in memory (in the normal case that they do overlap, Windows relocates them, but this frequently leads to fork errors in Cygwin).

This ticket solves the problem by running the sage-rebase.sh script as part of the sage-spkg script at the end of every package installation. It also runs sage-rebase.sh after make sagelib for the same reason. Although running it after installing each package is generally unnecessary, sometimes it is necessary. For example, after installing Python, it's necessary to rebase to ensure that Python works, as it will be used later in the build process. Since rebasing doesn't actually add much overhead in terms of time, it's fine to just do it always (on Cygwin).

Note that this also uses sage-rebase.sh versus sage-rebaseall.sh. The difference is that the former only updates DLLs in Sage itself (i.e. under $SAGE_LOCAL) whereas the latter also rebases all DLLs in the Cygwin installation. Because of this, the latter cannot be run from inside Cygwin--all Cygwin processes must be stopped first. Obviously this would be too disruptive to the normal Sage build process, so we don't do that. It's also unnecessary. As long as rebaseall is run once after installing Cygwin and all build dependencies, it will build a database of known DLLs and their base addresses and sizes, which can be referenced when rebasing new DLLs. Since building Sage doesn't affect any of the Cygwin system DLLs there's no need to rerun rebaseall during a build of Sage.

Fixing this is necessary for unattended Sage builds to succeed--otherwise the build will fail several times and require manual rebasing before proceeding.

Old Description

With Sage 5.12 (plus a few updated spkg, all of them available on trac), building Sage on Cygwin(32) is no harder than on Linux except for the need of rebasing a few times during the build process.
This could be automated in the following way

  • try to build
  • if that fails and we're on Cygwin,
  • grep the failing logs for error messages cuased by fork issues
  • if all erros are caused by fork issues:
    • rebase (using the oblivious option which can be run from bash, I think the script we included some time ago are ok),
    • go back to step one
  • if not fails as usual

Having this would open the way to a buildbot or a patchbot for Cygwin.

Depends on #20986

CC: @kcrisman @dimpase @jdemeyer @jpflori @tscrim

Component: porting: Cygwin

Author: Erik Bray

Branch/Commit: 5e494ad

Reviewer: Jean-Pierre Flori, Jeroen Demeyer

Issue created by migration from https://trac.sagemath.org/ticket/15423

@jpflori jpflori added this to the sage-6.1 milestone Nov 15, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@embray
Copy link
Contributor

embray commented Apr 13, 2017

comment:4

I've found it simpler (and I have a patch) to just always rebase after every package build. It doesn't actually add much noticeable overhead. One thing I might want to do before pushing my patch is also make it so the --quiet flag can be passed through to rebase, since otherwise this sometimes generates a bit of noise that isn't helpful.

I would say this is almost necessary for Cygwin builds; especially in an automated context like running the patchbot.

@embray embray modified the milestones: sage-6.4, sage-8.0 Apr 13, 2017
@embray embray self-assigned this Apr 13, 2017
@embray
Copy link
Contributor

embray commented May 5, 2017

comment:6

New method for rebasing DLLs continuously during Sage build, which should solve this issue. I've updated the ticket description to explain exactly what this does and why, but I left the old description just for comparison.


New commits:

fcccd28Include ECL .fas binaries (which are DLLs) when calling rebase(all)
6d5ad32These scripts don't necessarily need to know where SAGE_ROOT is, just SAGE_LOCAL
b5daf80Update these batch scripts so that it's not necessary to manually specify the paths of SAGE_LOCAL or CYGWIN_ROOT
23a5a04Accept the path to SAGE_LOCAL as an argument (this is useful for running outside the Sage shell).
e2f0b40Enhance sage-rebase.sh to be able to pass additional arguments to rebase, and to wrap the call to rebaseall
df53e2fThis should be passing the --all flag to sage-rebase.sh
f39f086Run sage-rebase between each spkg install on Cygwin
cae4bc7Also rebase after building sagelib itself
48ff8d8Make the rebase call after each package quiet.

@embray
Copy link
Contributor

embray commented May 5, 2017

Commit: 48ff8d8

@embray
Copy link
Contributor

embray commented May 5, 2017

Author: Erik Bray

@embray

This comment has been minimized.

@embray
Copy link
Contributor

embray commented May 5, 2017

Branch: u/embray/cygwin/ticket-15423

@embray
Copy link
Contributor

embray commented May 5, 2017

Dependencies: #20986

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 5, 2017

Changed commit from 48ff8d8 to e6297c3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 5, 2017

Branch pushed to git repo; I updated commit sha1. New commits:

e6297c3Move the sage-rebase.sh after sagelib installation to the Makefile for sagelib itself, rather than build/make/deps

@embray
Copy link
Contributor

embray commented May 5, 2017

comment:9

Looks like that last commit has a merge conflict.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 5, 2017

Changed commit from e6297c3 to ff8e807

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 5, 2017

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

42aeeaeInclude ECL .fas binaries (which are DLLs) when calling rebase(all)
b60823bThese scripts don't necessarily need to know where SAGE_ROOT is, just SAGE_LOCAL
e2905b1Update these batch scripts so that it's not necessary to manually specify the paths of SAGE_LOCAL or CYGWIN_ROOT
5d8910cAccept the path to SAGE_LOCAL as an argument (this is useful for running outside the Sage shell).
43eb7bfEnhance sage-rebase.sh to be able to pass additional arguments to rebase, and to wrap the call to rebaseall
784c0e2This should be passing the --all flag to sage-rebase.sh
747e201Run sage-rebase between each spkg install on Cygwin
6579d0aAlso rebase after building sagelib itself
8c2d48eMake the rebase call after each package quiet.
ff8e807Move the sage-rebase.sh after sagelib installation to the Makefile for sagelib itself, rather than build/make/deps

@jpflori
Copy link
Author

jpflori commented May 17, 2017

Reviewer: Jean-Pierre Flori

@jpflori
Copy link
Author

jpflori commented May 17, 2017

comment:12

LGTM

@jdemeyer
Copy link

comment:13

In src/Makefile.in, is this really needed for the rebase script to work?

    (cd $(srcdir)                                       \
     && export SAGE_ROOT=/doesnotexist                          \
           SAGE_SRC=/doesnotexist                           \
           SAGE_SRC_ROOT=/doesnotexist                          \
           SAGE_DOC_SRC=/doesnotexist                           \
           SAGE_BUILD_DIR=/doesnotexist                         \
           SAGE_PKGS=$(abs_top_srcdir)/build/pkgs                   \
           SAGE_CYTHONIZED=$(abs_builddir)/build/cythonized             \

If not, it would be cleaner to write the rebase script on a new line instead of continuing with &&.

Minor comment which you might as well fix: you don't need the parentheses (cd ... && ...). Those just create an extra subprocess for no reason.

@jpflori
Copy link
Author

jpflori commented May 17, 2017

comment:14

Nah the rebase script does only care about SAGE_LOCAL.

@jdemeyer
Copy link

Changed branch from u/embray/cygwin/ticket-15423 to u/jdemeyer/cygwin/ticket-15423

@jdemeyer
Copy link

Changed commit from ff8e807 to 5e494ad

@jdemeyer
Copy link

New commits:

5e494adReformat Makefile rule

@jdemeyer
Copy link

Changed reviewer from Jean-Pierre Flori to Jean-Pierre Flori, Jeroen Demeyer

@jpflori
Copy link
Author

jpflori commented May 18, 2017

comment:17

Ok, let's go with jeroen solution.

@embray
Copy link
Contributor

embray commented May 18, 2017

comment:18

You're right, this is fine.

@vbraun
Copy link
Member

vbraun commented May 20, 2017

Changed branch from u/jdemeyer/cygwin/ticket-15423 to 5e494ad

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants