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.

Sign up for GitHub

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

Jump to bottom

Add NSIS 2 #1089

Merged
merged 3 commits into from
Aug 6, 2016
Merged

Add NSIS 2 #1089

merged 3 commits into from
Aug 6, 2016

Conversation

jakirkham
Copy link
Member

@jakirkham jakirkham commented Jul 17, 2016 •
edited
Loading

Adds a recipe for NSIS 2.x. Adapted from this recipe by @faph. Note it is in the Public Domain. Feedback and Maintainers welcome.

Also there is a newer version (2.51) and it will be updated in the feedstock. Trying to match what is in the anaconda channel first and then upgrading from there.

cc @conda-forge/core @patricksnape @gillins @msarahan @mingwandroid

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/nsis) and found it was in an excellent condition.

This was referenced Jul 17, 2016
Merged
Closed
patricksnape
patricksnape reviewed Jul 18, 2016
View reviewed changes
recipes/nsis/bld.bat
@@ -0,0 +1,4 @@
robocopy . "%PREFIX%" /S /XF bld.bat
if errorlevel 8 exit 1
Copy link
Contributor

@patricksnape patricksnape Jul 18, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this should be not equal 1

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Changed.

@patricksnape
Copy link
Contributor

Currently failing due to the conda-build bug.

@jakirkham
Copy link
Member Author

The conda-build bug should now be fixed and is in the 1.21.7 release. Restarting to see if it now passes.

jakirkham
jakirkham reviewed Jul 25, 2016
View reviewed changes
recipes/nsis/meta.yaml
about:
home: http://sourceforge.net/projects/nsis
summary: Nullsoft Scriptable Install System
license: CPL-1.0, zlib, bzip2
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May want to include the license file for good measure. Then again it may already be in this source.

@jakirkham
Copy link
Member Author

jakirkham commented Jul 26, 2016 •
edited
Loading

Trying to change this to build from source, but it is a bit tricky. Feedback welcome.

@jakirkham jakirkham mentioned this pull request Aug 4, 2016
Closed
@patricksnape
Copy link
Contributor

Does this indicate we are still in the root environment?

@jakirkham jakirkham force-pushed the add_nsis branch 2 times, most recently from 75d8f07 to 449bb70 Compare August 5, 2016 23:02
@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/nsis) and found it was in an excellent condition.

@jakirkham
Copy link
Member Author

It is a bit strange. Though I'm see that with builds that are passing too.

msarahan
msarahan reviewed Aug 5, 2016
View reviewed changes
recipes/nsis/meta.yaml

build:
number: 0
skip: true # [unix]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason to build this with py34/35? Last I checked, scons was py2 only. I think the py34/35 builds are actually not producing good output.

@jakirkham jakirkham force-pushed the add_nsis branch 2 times, most recently from 3d8e2ea to 91b7d72 Compare August 5, 2016 23:35
@jakirkham
Copy link
Member Author

Yep, only added in that python stuff as I was concerned that I was missing something basic about how this should be built. Though it makes sense it shouldn't be needed as you have said @msarahan so it has now been dropped.

@jakirkham
Copy link
Member Author

I think we need to set the TOOLSET environment variable, but am not sure what it should be set to. This is based on diving into their SConstruct file.

@jakirkham
Copy link
Member Author

Also seems they are ignoring every environment variable except PATH. 😕

@jakirkham jakirkham force-pushed the add_nsis branch 6 times, most recently from b4e16c5 to 4f6e4cb Compare August 6, 2016 02:04
@jakirkham
Copy link
Member Author

Reverting back to working binary install. As nice as it would be to build this from source, it is proving challenging and I'm not feeling up to the task. Will add it as an issue in the feedstock.

jakirkham
jakirkham reviewed Aug 6, 2016
View reviewed changes
recipes/nsis/meta.yaml
recipe-maintainers:
- jakirkham
- msarahan
- patricksnape
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added both of you here @msarahan and @patricksnape. Please let me know if this is ok.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@msarahan, I went ahead and self-merged this (after some discussion). If you decide that you don't want to be a maintainer, please let me know.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Fine with me. Happy to help where I can.

@jakirkham jakirkham changed the title WIP: Add NSIS 2 Add NSIS 2 Aug 6, 2016
@patricksnape
Copy link
Contributor

Yup - let's just add it as an issue and get this in as a copy for now. 😄

@jakirkham jakirkham merged commit 449d984 into conda-forge:master Aug 6, 2016
@jakirkham jakirkham deleted the add_nsis branch August 6, 2016 17:45
@jakirkham
Copy link
Member Author

Thanks @patricksnape. I've added issue ( conda-forge/nsis-feedstock#1 ) to the feedstock so we can discuss how to get this built properly there.

@jakirkham
Copy link
Member Author

@faph, just letting you know that we have added nsis to conda-forge. This will allow users to easily install this with conda ( conda install -c conda-forge nsis ). The maintenance burden here is light as all releases are built and deployed automatically with CI. Changes to how it is being built can be controlled by changing the recipe in this feedstock (repo). If you have any interest in being added as a maintainer, we would be more than happy to add you to the maintainers list. Thanks for your work on packaging nsis for conda.

@faph
Copy link
Contributor

faph commented Aug 9, 2016

No worries. I'm currently not using NSIS very much at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jakirkham @conda-forge-linter @patricksnape @faph @msarahan