Open bpancras opened 8 months ago
vijayiyer05
commented
6 months ago Hi @bpancras, looks like @kleinerm has force-pushed this PR, which I didn't catch earlier. Is there additional information/action you may need/want from the PTB side? I'm not as familiar with force-pushes.
kleinerm
commented
5 months ago Hi @bpancras and @vijayiyer05
sorry for the late reply. I was forced by my boss, without meaningful heads up, to use up all saved up remaining vacation days of the last years 2022-2023, of which there were many, or lose all of them without any compensation. This lead to a mostly complete stand-still of most not-super-critical work on PTB since early March, and now to being behind in schedule by over 1.5 months on important projects. Hence this delay, and likely future delays in the next months, sorry.
Anyhow, I just had a quick skim over this, and while I thank you for trying, this contribution is not suitable in its current form.
As stressed in earlier e-mails to you from many months ago, sometimes last year, and in the online meeting we once had, any such installer must also reliably work with GNU/Octave, version 5.2 or later at this point, not only with the most recent versions of Matlab. Otherwise it is a big step back for us and just increases confusion of our users and software-maintenance and user-support overhead. Even the latest Octave does not support the websave() function yet, and webread() is not available in Octave 5.2 - shipping with Ubuntu Linux 20.04 and similar, so even webread() can not be used until we drop support for Octave 5.x completely. I don't know about the status/capabilities/compatibility of the used inputParser() function either. Anything that uses OOP functionality may be shaky in some cases. There may be other troublemakers. Use of a closed-source function like getAddOnsFolder.p is also not acceptable and certainly permanently incompatible with Octave.
I don't see sufficient error handling and guidance and help for users. Especially installer/setup functionality must be extra careful.
As we don't want new installer functions in the first place, whatever is done would have to be implemented inside the existing DownloadPsychtoolbox.m and - if applicable - UpdatePsychtoolbox.m functions, which already provide help texts, user guidance, relatively careful error handling and all the post-install routines and mechanisms. This is what users are used to from the past. Ideally we'd have a replacement for the old Subversion based install/update methods, just directly based on Git, but failing that, something that is compatible with Octave. E.g., the older urlread() and urlwrite() functions seem to be supported by both Octave and Matlab, although compatibility would have to be tested and taken into account.
If you look at the svndownload() subfunction inside DownloadPsychtoolbox.m and its use and implementation, I think that would be roughly what would need to be reimplemented, ideally via Git access, or via urlread/urlwrite variants of your approach. The function as a whole will need some cleanup, but the meat of it is a svndownload subfunction replacement that is fully compatible with Octave 5.2 and later and Matlab R2014b and later.
kleinerm
commented
4 months ago A few more comments after playing with bits of code under Octave 8.4. I assume stuff works hopefully the same with Octave 6.1 and later, which would be required.
webread() is supported in Octave 6.1 and later, which would be good enough for our purposes. The way to get releaseJSON on Octave, as used in your code, would be via releaseJSON = jsondecode(webread(...)); instead of releaseJSON = webread(...);, cfe. IsOctave() function for detecting Octave vs. Matlab to choose the right branch in an if-else-end for this. However, jsondecode only exists on Octave 7.1, not on Octave 6.1 which we still need to support for Ubuntu 22.04, so that's a problem.
urlwrite() works as replacement for webwrite() in Octave.
You would not want to store the zip file in the Matlab Add Ons folder in the first place, as no such thing exists on Octave or in older Matlab's, and .mltbx files installed via the Add Ons explorer would be the way to go anyway in that case. So that whole logic can go and one uses the targetfolder specified by the user in DownloadPsychtoolbox.m etc.
Finally, that's the wrong zip file to use, as it contains also all the C-Source, build scripts, Python code etc. You'd want the 3.0.x.y.zip file, e.g., from the url:
downloadurl = releaseJSON{i}.assets(2).browser_download_url;
Btw. for some unfortunate reason, the mltbx files also seem to contain the source folders etc., despite them not being specified in the toolbox project file. Only the Psychtoolbox/ subfolder should be part of those.
This contribution implements a new installation workflow for Psychtoolbox centered around GitHub's Rest API. This is a prototype implementation targeted towards replacing the now deprecated SVN based installation workflow. The implementation has the following interface:
The installPTB script can be used until version 3.0.16.8, the tagging convention was different before this release.
"THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY."