Odpowiedzi

I tested this on Wine 1.7.44 and 1.6.2 (which is what the script was tested on). 1.7.44 has a lot of graphic errors, but switching to 1.6.2 fixes them. This is why I personally think declaring a WINEVERSION should be mandatory, and leave it up to the actual user to use the system WIne if they choose. My system Wine causes all sorts of graphic issues, but since we know that 1.6.2 works fine, you can be certain that everyone uses that Wine version, since their system Wine could literally be anything (vanilla Wine of any version, patched, wine-staging, etc.).

I recommend declaring WINEVERSION="1.6.2" and using that variable in the script when invoking Wine. That way users with versions other than that don't have to worry (which is one of the great parts about POL). As of now, this could be glitchy for any user not using 1.6.2, which is easily taken care of through POL. :)

Ok. I had just never seen them used as such, and tested it fine without the double brackets. Just trying to help. Sorry.

That being said, I still stand by my statement about the Wine versions (it tested glitchy on my system Wine, but using the specific one it was tested with worked fine) being declared. From a compatibility standpoint, there is no reason not to since it can easily be done, but yeah. I won't bring it up anymore.

Ronin, I've noted some similar issues - I intend to do further testing, and narrow down the range of versions it is broken on. At very least, it'd be nice to figure out what kind of bug to send upstream to Wine. In the meantime, I'll see about going for 1.6.2 in the next version.

Yes; I highly recommend simply declaring a WINEVERSION variable with 1.6.2, then you never have to worry about it again (2 lines of code). Basically, right now, anyone on Arch will see glitches, because Arch does not use the stable Wine versions on it's system Wine (always uses the newest releases); basically, as the script stands, it is broken on pretty much any Arch-based system until that variable is declared and used. That is why I suggest it always be included, as it eliminates compatibility problems, and is one of the main features of POL. I am sorry for being redundant. It's just that this whole script is essentially broken for a whole series of distros, just because of 2 lines of code. haha. To me, it is an "elephant in the room", so to speak. It also makes it one less thing you have to troubleshoot (because users will report problems here, and that is one problem that can be taken care of before it is actually a problem).

Yes, It's likely this POL script will go with WINEVERSION=1.6.2 for now. However, as someone who is a sysadmin for $DAYJOB as well as having done distro packaging, I really do have to view it as a partial solution at best. Version pinning is suboptimal because while it's often billed as avoiding bugs in new versions, it also does two deeply suboptimal things: avoids fixes in new versions, and prevents regressions in new versions from being found. The former often causes breakage when, say, the program a script is for begins using new features. And the latter means such regressions are generally not caught until it's been too much time to easily track down (or fix) the source of the error.

Eventually, 1.7 will become 1.8, and any regressions that hadn't been caught will inflict themselves even on those who stick with stable.

Honestly, I'd be much more sanguine if WINEVERSION was instead WINEREQUIREMENT, which specified a list of constraints along the lines of "<=1.7 | (>1.7.30 & < 1.7.50)", and tried to satisfy it with the newest valid version.

Either way, you are declaring a Wine version that has tested working, instead of relying on the system Wine version, which you have no control over.

As someone who has written a bunch of scripts and tested them on quite a few systems, declaring a Wine version (and simply keeping it up to date when it needs to be) negates any issues. Do whatever you want. lol. You will have to troubleshoot the issues that arise. :)

WINEVERSION is the required, working version for Wine. All it is doing it telling the script to download the version of Wine required for it to work. There is no need to over-complicate it. Simply declaring a Wine version variable and using it will tell POL to do the rest (it is all automated).

In a perfect world, all programs would work through the newest Wine version. That, however, is not the case; that is where POL comes in. It can download and assign a Wine version per-application. If it is tested correctly, it will continue to work. If the program is updated, and issues arise, then users should report the bugs so that, if needed, the Wine version in the script can be updated.

Yes, it is a partial solution, because Wine is not perfect. If it was, POL would not exist. :)

Yup, constraints system has been envisioned, getting it right is something different though...
A middleground solution could also be to have pseudo-versions say $WINE_LATEST_STABLE and $WINE_LATEST_UNSTABLE (maybe $WINE_LATEST_STAGING now too), filled by the client from data published by the web server...
Some of the ideas I've been toying around for some time already ;)

That program requires a specific version of Wine with a specific patch. If you were to rely on the system Wine, that program would not work at all, and the user would be required to find the version of Wine needed, find the patches, download the source, compile and install it, and then run the script. Declaring a WINEVERSION eliminates that need, and makes it easier for users and far, far more compatible across multiple systems. Again, it is up to you; just trying to make less work for you.

Ronin, I _did_ say the script will likely go with a WINEVERSION. I was just expanding on some of my reasoning for not being as excited/optimistic about the benefits as you seem to be.

petch, a constraint system could be done relatively simply if you follow the Haskell example of a list of non-overlapping ranges - checking if a version is acceptable is then just a matter of (for each version in range, if version in range, return true, done, return false). This can, in fact, specify any set of valid versions, so it loses no expressive power.

A very simple syntax might be splitting ranges on whitespace (or comma), splitting left and right on ".." (will never be in a Wine version), and treating standalone versions as single-element ranges. Inclusive ranges would likely work best in terms of semantics: the constraint would just be lowest_tested..highest_tested in many cases.

" I was just expanding on some of my reasoning for not being as excited/optimistic about the benefits as you seem to be."

Um, not excited/optimistic. I KNOW it works better from personal experience. IMO, it is lazy and sloppy to not utilize one of the main features that almost every single script uses to make it more compatible for the users that will be trying to install it.

There are hundreds of Wine versions.... how would you compile testing data? Really, that seems immensely more complicated than just telling it what version to use and updating as reports come in. In programming, being concise and explicit is better than assumption. At least from my experience.

yes, patterns could be interesting too, say 1.6.* if you know that any of those versions do work (and hope that any 1.6.3 would be a bugfix-only release and probably work too).
But syntax is not the only issue here, you have to consider the policies, ie if several already installed Wine versions match, which one to pick? If none match, which one to download? (and favor reuse).
As new versions of Wine are released, should the client attempt something?
And then you wonder if all this is not overkill, because most likely you'll be in two cases:
- programs that seem to work with the recent versions of Wine, and picking the latest available is probably the best course of action. If some new version breaks it you'll have to, and be able to, set the version to use by hand. The other case being programs that already do not work best with recent versions, so same case.

I just tested in on wife's system, too. Mint with wine-staging 1.7.44 installed as the system Wine... script results in glitchy program. Switching to 1.6.2 fixes it. Same bug, two systems, fixed by a declared Wine version. ;) I go play video games now. lol.

Odpowiedzi

Missed how to upload the source code the first time around. Sorry. Also, accidentally typoed the name - "PattenMaker" where it should be "PatternMaker" - I'm not seeing a way to edit this; am I missing something obvious?

First, nice arrays and associate arrays work :)
Ok, now for the review:
POL_SetupWindow_checkbox_list "What patterns would you like to install?" "${TITLE}" "${PATTERN_NAMES[*]}" "~"
All user-oriented messages (here, the question) should support localization, see http://wiki.playonlinux.com/index.php/Scripting_-_Chapter_10:_Script_Translation

# Create the environment if it does not yet exist
if ! POL_Wine_PrefixExists; then
POL_Wine_PrefixCreate
fi
PrefixCreate will already ask the user whether (s)he'd like to overwrite or erase an existing prefix (since 4.1.9), so this just makes it more complicated for users to reinstall from scratch, I'm not convinced this is an improvement.
And the test is wrong, it should be if [ "$(POL_Wine_PrefixExists $PREFIX)" != "True" ]; then ...

# Download the installer, verifying it against a digest
POL_Download_Resource "${INSTALLER_URL}" "${INSTALLER_DIGEST}" "${TITLE}"
Since the resources directory is totally hidden from user eyes (no report of how much disk space it takes, no visible way to clear it), as a general rule it has been used only for files reusable between scripts, and mainly for components.

Anyway, thanks for the review! The full script is actually generated from a template and per-component snippets, which is why the arrays are a thing. I'll definitely fix up the localization bit.

I'll also remove the PrefixExists check; it was a holdover from a prior version where I tried doing separate installers for PatternMaker (the tool itself) and other sub-tools or data installers. Moving to a single meta-installer has basically obviated the reason it was added.

I use POL_Download_Resource for a few reasons:
1.) Re-running the installer to install more of the patterns is a use case we explicitly intend to support, and redownloading the main installer for that is a bit annoying.
2.) The "patterns" really are components (actually data files used by PatternMaker), and the user may very well reuse them in a subsequent run of the tool (or when they update the PatternMaker main program itself)
3.) POL_Download seems to go into /tmp, which on a number of distros is a tmpfs.

If you'd like, though, perhaps adding a yes/no on "Keep downloaded files after installation has finished?" and deleting them in the script on "no" would suffice?

SetNativeExtension looks perfect, no idea how I missed that!

And I'll definitely add the exit 0; for some reason I thought the GPG sigs would be in detached mode.

"3.) POL_Download seems to go into /tmp, which on a number of distros is a tmpfs."

You have to run POL_System_TmpCreate "$TITLE" to create a temp folder in POL's root folder, you can cd to it with cd "$POL_System_TmpDir", and clean it up after you are done with POL_System_TmpDelete. :)

Ah, thanks Ronin. Still, the fact that the components/installer do have real use cases for reuse makes me more tempted to use ..._Resource's subdir support (which the script already does via $TITLE), and give the user the choice of removal or preservation.

POL_Download downloads in the current directory, what about downloading in the virtual drive instead? So the file will be around for adding software components for as long as the virtual drive is not totally erased...

petch, the problem with that is that these are installers, just ones which may be useful to keep around. If the user is doing a clean reinstall (exactly where they'd benefit!), they'd choose to wipe the prefix on re-running the script... eliminating the downloaded files.

TITLE="PatternMaker"
PREFIX="PatternMaker"
# Note: This is assumed to end in /INSTALLER_NAME
INSTALLER_URL="http://www.patternmakerusa.com/downloads/PatternMaker7_5_Setup.exe"
INSTALLER_DIGEST="f2625e0b06a58cddf026abf873a58ed2"
MACROTOOL_URL="http://www.patternmakerusa.com/downloads/MacroGen4_5_Setup.exe"
MACROTOOL_DIGEST="c54cf396c9e53135eeb1fefc7cfd9201"