-
Notifications
You must be signed in to change notification settings - Fork 392
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
Initial Python App Packaging #10716
Initial Python App Packaging #10716
Conversation
|
I will wait on #10744 to merge in to get the release packages mostly fixed up then pull develop in here and get a release package tested from this branch. Following commits here will vary depending on the results of that test... |
if(WIN32) | ||
# on Windows, with Build Package, and also Link With Python, we can also drop in shortcuts to the new auxiliary CLI | ||
# NOTE: The install command COMPONENTS should line up so that they are run at the same packaging step | ||
install(CODE "execute_process(COMMAND powershell.exe -ExecutionPolicy Bypass -File ${PROJECT_SOURCE_DIR}/scripts/dev/create_shortcut.ps1 -TargetPath \"$<TARGET_FILE:energyplus>\" -ShortcutPath \"$<TARGET_FILE_DIR:energyplus>/EPLaunchPython.lnk\" -Arguments \"auxiliary eplaunch\")" COMPONENT Auxiliary) | ||
install(FILES $<TARGET_FILE_DIR:energyplus>/EPLaunchPython.lnk DESTINATION "./" COMPONENT Auxiliary) | ||
endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An interesting choice to call a powershell script to create it in the the installed package.
I suppose this could be helpful even in the tar.gz installer.
I would have expected a modification or an addition around here for the Start Menu though, no?
component.addOperation("CreateShortcut", "@TargetDir@/EP-Launch.exe", target_dir + "/EP-Launch.lnk"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, we may want an x-energyplus.xml and a eplaunch.desktop file for debian?
and adjusting or adding to
EnergyPlus/cmake/qtifw/install_registerfiletype.qs
Lines 52 to 54 in 48aa15b
component.addElevatedOperation("RegisterFileType", "idf", `${targetDir}\\EP-Launch.exe %1`, "EnergyPlus Input Data File", "text/plain"); | |
component.addElevatedOperation("RegisterFileType", "imf", `${targetDir}\\EP-Launch.exe %1`, "EnergyPlus Input Macro File", "text/plain"); | |
component.addElevatedOperation("RegisterFileType", "epg", `${targetDir}\\EP-Launch.exe %1`, "EnergyPlus Group File", "text/plain"); |
x-energyplus.xml would probably be something like
<?xml version="1.0" encoding="UTF-8"?>
<mime-info xmlns="http://www.freedesktop.org/standards/shared-mime-info">
<mime-type type="application/x-energyplus">
<sub-class-of type="text/plain"/>
<comment>EnergyPlus Input Data File</comment>
<comment xml:lang="fr">Fichier d'entrées EnergyPlus</comment>
<glob pattern="*.idf" />
</mime-type>
//// deal with imf and epg
</mime-info>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, launchers for Mac/Linux would be nice, but I think we'll leave that for another day. Let's let the interested parties run it on Windows and get things polished up before throwing out too many launchers. :)
release/RunEPLaunch.bat
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed?
// #if LINK_WITH_PYTHON | ||
// // static constexpr std::array<std::string_view, 6> logLevelStrs = {"Trace", "Debug", "Info", "Warn", "Error", "Fatal"}; | ||
// | ||
// auto *auxiliaryToolsSubcommand = app.add_subcommand("auxiliary", "Run Auxiliary Python Tools"); | ||
// auxiliaryToolsSubcommand->require_subcommand(); // should default to requiring 1 or more additional args? | ||
// std::vector<std::string> python_fwd_args; | ||
// | ||
// auto *epLaunchSubcommand = auxiliaryToolsSubcommand->add_subcommand("eplaunch", "EP-Launch"); | ||
// epLaunchSubcommand->callback([&state, &python_fwd_args] { | ||
// fs::path programDir = FileSystem::getParentDirectoryPath(FileSystem::getAbsolutePath(FileSystem::getProgramPath())); | ||
// fs::path const pathToPythonPackages = programDir / "python_lib"; | ||
// fs::path const tclLibraryDir = pathToPythonPackages / "tcl8.6"; // TODO: Find this dynamically | ||
// std::string tclLib = "TCL_LIBRARY=" + tclLibraryDir.string(); | ||
// | ||
// putenv(tclLib.c_str()); | ||
// fs::path const pathToEPLaunchExe = pathToPythonPackages / "bin" / "energyplus_launch.exe"; | ||
// system(pathToEPLaunchExe.string().c_str()); | ||
// | ||
// | ||
// exit(0); | ||
// }); | ||
// | ||
// #endif | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this commented out?
Alright, so Mac is struggling with this. The addition of codesigning and notarizing, with the addition of bringing along more Python dependencies, is causing too much interference. @jmarrec has been beyond helpful, but I'm not quite comfortable throwing it all in...at least not today. This is working beautifully on Windows and Linux, and if you do some manual shenanigans after install, EP-Launch is actually working on Mac. But getting those manual shenanigans into our build process is another story, that needs attention that I can't give right now. So I'm going to polish up what's here, and temporarily disable it on Mac, and get this merged. If, over the next 1-1.5 weeks, this can get to a happy place on Mac, we'll go ahead and add it back in. |
Alright, this seems good. I'm going to merge this in and get RCs tagged today and hopefully release tomorrow. |
Pull request overview
energyplus.exe auxiliary eplaunch
. This is great because the user does not have to install Python to get EP-Launch. I will comment that it's probably a cleaner/easier to maintain approach to just install your own Python and pip install the tools you want, but this will still help some users and start getting the EnergyPlus Python Apps out into the wild.energyplus-api-helpers
package. This is nice because the user can use their own Python environment/packages. And this is already available, nothing to do with this.