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

[WIP] Unpack idaes-local tar file in get-extensions command #1276

Closed
wants to merge 2 commits into from

Conversation

Robbybp
Copy link
Member

@Robbybp Robbybp commented Oct 2, 2023

Fixes

Part of IDAES/idaes-ext#261

Summary/Motivation:

Make CyIpopt as easy to use as possible.

Changes proposed in this PR:

  • After unpacking idaes-solvers-<os>.tar.gz into .idaes/bin, unpack idaes-local-<os>.tar.gz (which is included in idaes-solvers) into .idaes

Opening as a WIP as there are some unresolved details:

  • Current implementation assumes that the parent directory of the "install-to directory" is writable (and the correct place to unpack idaes-local). I'm not sure what the best way to specify this directory is.
  • Implementation could probably be streamlined if we distributed idaes-local-<os>.tar.gz on the downloads page of idaes-ext (instead of inside the idaes-solvers tar file)
  • We currently distribute idaes-local for MacOS as idaes-local-darwin-arm64.tar.gz, which is inconsistent with all the other tar files distributed for MacOS (which are *-darwin-aarch64.tar.gz). I would recommend changing this in idaes-ext

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

@Robbybp Robbybp marked this pull request as draft October 2, 2023 23:34
@ksbeattie ksbeattie added the Priority:Normal Normal Priority Issue or PR label Oct 5, 2023
@ksbeattie
Copy link
Member

ksbeattie commented Nov 2, 2023

@Robbybp is this expected for the Nov release? If so we should find a time to coordinate it, if you can't make it to the dev calls.

@Robbybp
Copy link
Member Author

Robbybp commented Nov 3, 2023

I'm going to close this for now. Once IDAES/idaes-ext#262 is merged, we will need to cut an idaes-ext release. At approximately the same time, I will re-open this PR (and probably combine it with an idaes install cyipopt command). While this PR could go forward as-is, it will need to be rewritten once IDAES/idaes-ext#262 is merged, and the benefit of unpacking the idaes-local tar file will not be realized until then either.

@ksbeattie As to whether this is expected for the Nov release, it depends on how long it takes us to get IDAES/idaes-ext#262 merged. My preference would be to get this in, but I will be at a conference next week and probably not able to work on IDAES/idaes-ext#262 until I'm back, so I'd say it's a toss-up.

@Robbybp Robbybp closed this Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:Normal Normal Priority Issue or PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants