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

Set up nasm from external.bat #61917

Closed
jpe mannequin opened this issue Apr 13, 2013 · 12 comments
Closed

Set up nasm from external.bat #61917

jpe mannequin opened this issue Apr 13, 2013 · 12 comments
Assignees
Labels
build The build process and cross-build OS-windows type-feature A feature request or enhancement

Comments

@jpe
Copy link
Mannequin

jpe mannequin commented Apr 13, 2013

BPO 17717
Nosy @pitrou, @tjguk, @zware, @zooba
Files
  • issue17717-default.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/zware'
    closed_at = <Date 2014-11-01.23:57:31.283>
    created_at = <Date 2013-04-13.16:34:32.806>
    labels = ['type-feature', 'OS-windows', 'build']
    title = 'Set up nasm from external.bat'
    updated_at = <Date 2014-11-01.23:58:27.644>
    user = 'https://bugs.python.org/jpe'

    bugs.python.org fields:

    activity = <Date 2014-11-01.23:58:27.644>
    actor = 'zach.ware'
    assignee = 'zach.ware'
    closed = True
    closed_date = <Date 2014-11-01.23:57:31.283>
    closer = 'zach.ware'
    components = ['Build', 'Windows']
    creation = <Date 2013-04-13.16:34:32.806>
    creator = 'jpe'
    dependencies = []
    files = ['37089']
    hgrepos = []
    issue_num = 17717
    keywords = ['patch']
    message_count = 12.0
    messages = ['186752', '187490', '187543', '187571', '222922', '230090', '230251', '230353', '230359', '230360', '230471', '230473']
    nosy_count = 7.0
    nosy_names = ['jpe', 'pitrou', 'tim.golden', 'BreamoreBoy', 'python-dev', 'zach.ware', 'steve.dower']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue17717'
    versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

    @jpe
    Copy link
    Mannequin Author

    jpe mannequin commented Apr 13, 2013

    It would be nice for Tools\buildbot\external.bat to set a copy of nasm up to use. Is there a reason this is not done?

    @jpe jpe mannequin added OS-windows type-feature A feature request or enhancement labels Apr 13, 2013
    @zware
    Copy link
    Member

    zware commented Apr 21, 2013

    Could you elaborate on what you mean to be done? All I've ever had to do was run the nasm installer and add the install location to PATH.

    @jpe
    Copy link
    Mannequin Author

    jpe mannequin commented Apr 22, 2013

    What I'd like is for external to set up all the dependencies needed to build python and run the test suite. Yes, nasm can be downloaded and set up separately, but that's true of all of the libraries that external.bat downloads.

    @zware
    Copy link
    Member

    zware commented Apr 22, 2013

    I agree it would be nice, but I'm not sure how easy or practical it would be to implement, particularly making sure that NASM is on the PATH. And besides, at some point, we have to draw the line between what we can reasonably do for a user and what we can reasonably expect a user to do for themselves. That line falls at minimum after installation of Visual C++ 2010, which would be nearly impossible to properly install from a batch script, and is currently also after installation of NASM and Perl, which are both optional anyway--you can build Python without NASM, you'll just have build errors and won't have SSL support. I think that's a reasonable place to keep the line: short of trying to install external programs for the user.

    I think it would be good to have a nice error/warning message in build.bat if NASM (or MSVC++, for that matter) can't be found, possibly with a URL pointing to where to look for an installer. Something along the lines of:

    """
    where nasm >nul 2>&1

    if %ERRORLEVEL% == 1 (
    echo NASM not found on PATH. It can be downloaded from www.nasm.us
    set /P _continue=Continue without NASM? (y/n)
    if %_continue% == n exit /B 1 else echo Continuing...
    )
    """

    Does that come anywhere close to scratching your itch?

    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Jul 13, 2014

    I think a message as suggested in msg187571 would be more than adequate.

    @zooba
    Copy link
    Member

    zooba commented Oct 27, 2014

    Practically this is very easy to do, and I'm more than willing to author detection into the new PCbuild files.

    Having nasm mirrored on svn.python.org (or anywhere on a PSF host) would be real nice though. I don't particularly like making the build system rely on potentially unreliable external sites. Not sure what the legal ramifications here are though...

    @pitrou
    Copy link
    Member

    pitrou commented Oct 29, 2014

    NASM seems BSD-licensed, so it shouldn't be a legal problem.

    @zware
    Copy link
    Member

    zware commented Oct 31, 2014

    Ok, I've imported nasm-2.11.06 to:

    http://svn.python.org/projects/external/nasm-2.11.06

    I'll work on a patch for default and see what (if anything) will need to change in the openssl checkout.

    @zware
    Copy link
    Member

    zware commented Oct 31, 2014

    Fairly simple patch for default.

    @pitrou
    Copy link
    Member

    pitrou commented Oct 31, 2014

    I can't say anything about the patch, but thank you for automating this! One less manual step :-)

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 1, 2014

    New changeset 28d18fdc52c4 by Zachary Ware in branch '2.7':
    Issue bpo-17717: Pull NASM from svn.python.org for OpenSSL build.
    https://hg.python.org/cpython/rev/28d18fdc52c4

    New changeset f7ed3e058fca by Zachary Ware in branch '3.4':
    Issue bpo-17717: Pull NASM from svn.python.org for OpenSSL build.
    https://hg.python.org/cpython/rev/f7ed3e058fca

    New changeset ef15b51d59fb by Zachary Ware in branch 'default':
    Issue bpo-17717: Pull NASM from svn.python.org for OpenSSL build.
    https://hg.python.org/cpython/rev/ef15b51d59fb

    @zware
    Copy link
    Member

    zware commented Nov 1, 2014

    The patches for 2.7 and 3.4 were more trivial than for default, and I was pretty confident in the patch for default, so I went ahead and committed. I did switch around which end of PATH our copy of NASM was added to, to make it easier for someone to override which NASM was used.

    Thanks for the suggestion John, and thanks for the support Antoine!

    @zware zware closed this as completed Nov 1, 2014
    @zware zware self-assigned this Nov 1, 2014
    @zware zware added the build The build process and cross-build label Nov 1, 2014
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    build The build process and cross-build OS-windows type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants