-
Notifications
You must be signed in to change notification settings - Fork 13
Add embedded tar script packager #115
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
base: master
Are you sure you want to change the base?
Conversation
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.
Thanks, interesting idea. Sorry for the delay. I've not got a lot of time for NetBeans things at the moment, but had a glance through. Few concerns, but nothing major.
We were discussing getting a first non-beta release out, which involves removing a few deprecated options first. Not sure if this should merge before or after that release.
I was wondering whether we could reduce duplication of some of these options by having some shared Linux options. That might require some thought given the current structure, and might not be worth the trouble. Depends if we might expect more Linux package types in future.
| * @param template script template | ||
| * @return script with unescaped variables | ||
| */ | ||
| public static String unescapeScriptVars(String script) |
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.
I think this should be removed. It should be unnecessary. The API should give you the option to leave unmatched tokens in place already.
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.
This is still needed to avoid replacing bash variables that are used when the end user is using the installer script to install the software. Since the user is allowed to choose where to install the software some variables need to be interpreted as bash variables at runtime. e.g.
LAUNCHER=${{INSTALL_PATH}}/launcher/${APP_DIR} gets replaced with LAUNCHER=${INSTALL_PATH}/launcher/app_dir in the installer script.
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.
Sure, I understand what you're trying to do, but that should happen anyway without this extra code as long as the token is not present during packaging - eg. see the test at
netbeans-nbpackage/src/test/java/org/apache/netbeans/nbpackage/ExecutionContextTest.java
Lines 77 to 79 in 51e9d3d
| text = "${package.name} execution token ${EXEC}."; | |
| processed = ctxt.replaceTokens(text); | |
| assertEquals("Apache NetBeans execution token ${EXEC}.", processed); |
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.
My mistake, I did not realize only a preset list of tokens are replaced. I'll remove it and update the templates.
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.
Yes, there's StringUtils::replaceTokens where you want to leave any unmatched tokens as they are, and StringUtils::replaceTokensOrFail for where all tokens must be pre-defined. It was done like this to allow shell scripts and other contexts where the same ${...} is needed.
Ideally, think about the naming in the script templates to make it clear which are replaced at build time though if you need both. Maybe the additional tokens like APP_DIR should be lower case in this one? Or make sure they all start with APP_?
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.
Agreed. I've switch to lowercase for local bash variables, used package.tar.* for tokens, and _var_ for sed regex replacements.
src/main/java/org/apache/netbeans/nbpackage/shell/TarScriptPackager.java
Outdated
Show resolved
Hide resolved
src/main/resources/org/apache/netbeans/nbpackage/shell/Messages.properties
Outdated
Show resolved
Hide resolved
src/main/resources/org/apache/netbeans/nbpackage/shell/shell.script.template
Outdated
Show resolved
Hide resolved
src/main/resources/org/apache/netbeans/nbpackage/shell/shell.desktop.template
Outdated
Show resolved
Hide resolved
d8b6cfc to
725b710
Compare
Adds an embedded tar script packager.
~/.local/appdir/usr/local/appdir