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

Enhancements to sage-rebase scripts #20986

Closed
embray opened this issue Jul 8, 2016 · 12 comments
Closed

Enhancements to sage-rebase scripts #20986

embray opened this issue Jul 8, 2016 · 12 comments

Comments

@embray
Copy link
Contributor

embray commented Jul 8, 2016

The scripts sage-rebase(all).{sh,bat} are used when developing/building Sage on Windows.

In the long term I'd like to find a way to reduce the need for them, but even then they're still good to have. It is useful to run these every now and then to ensure that all DLLs in the Sage distribution are configured to load into non-overlapping address ranges by default. First and foremost this is a performance issue, but also not rebasing can cause failures with Cygwin's fork.

To summarize what this ticket changes:

  • In the wrapper batch scripts, it's no longer necessary to manually adjust CYGWIN_ROOT and SAGE_ROOT. In fact, SAGE_ROOT is not needed by these scripts so much as SAGE_LOCAL, the path to which is automatically detected from the location of the script. CYGWIN_ROOT is determined from the registry. This may be incorrect if there are multiple Cygwin installations on the system, but this is unlikely in most cases. Furthermore, with the changes I'm making for Add auto-rebasing mechanism for Cygwin #15423 there will be less need to use the batch script in the first place (I have rarely had to use them in a long time now).
  • In the sage-rebase.sh and sage-rebaseall.sh scripts:
    • Now the implementation is all just in sage-rebase.sh, which has grown an --all flag for performing rebaseall. So sage-rebaseall.sh is just a wrapper around sage-rebase.sh --all. This reduces a lot of code duplication.
    • The script excludes anything under /var/tmp so that build artifacts don't take up space in the address space layout.
    • The script does now include .fas modules for ECL, which were previously not included in the rebase.
    • It is possible to pass the path to $SAGE_LOCAL to the scripts as an argument, so it's not necessary for $SAGE_LOCAL to be set before running them (useful sometimes during development).
    • It is possible to pass arbitrary additional flags to the underlying rebase/rebaseall commands.

This is just a small collection of enhancements to these scripts. Most importantly, this adds support for rebase .fas binaries that are part of ECL. Not doing this potentially caused problems for Maxima (so I'm marking this as a defect).

CC: @jpflori

Component: porting: Cygwin

Keywords: cygwin windows

Author: Erik Bray

Branch/Commit: df53e2f

Reviewer: Jean-Pierre Flori

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

@embray embray added this to the sage-7.3 milestone Jul 8, 2016
@embray
Copy link
Contributor Author

embray commented Jul 8, 2016

comment:2

It's also "good" to be reminded every now and then how awful DOS batch scripts are and how lucky we are to have bash...

@embray
Copy link
Contributor Author

embray commented Jan 13, 2017

comment:3

I've made several more updates to these scripts that should be incorporated into this ticket.

@embray embray self-assigned this Apr 13, 2017
@embray embray modified the milestones: sage-7.3, sage-8.0 Apr 13, 2017
@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:

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 5, 2017

Changed commit from c4c0bae to e2f0b40

@embray
Copy link
Contributor Author

embray commented May 5, 2017

comment:7

I think I've done all I need with these for now.

@embray

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 5, 2017

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

df53e2fThis should be passing the --all flag to sage-rebase.sh

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 5, 2017

Changed commit from e2f0b40 to df53e2f

@jpflori
Copy link

jpflori commented May 17, 2017

Reviewer: Jean-Pierre Flori

@jpflori
Copy link

jpflori commented May 17, 2017

comment:9

LGTM.

@embray
Copy link
Contributor Author

embray commented May 18, 2017

comment:10

Thanks!

@vbraun
Copy link
Member

vbraun commented May 19, 2017

Changed branch from u/embray/cygwin-rebase-fas to df53e2f

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

3 participants