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

Use Python3 piggy packing the Linux binary #3655

Merged
merged 1 commit into from
Jan 21, 2021
Merged

Use Python3 piggy packing the Linux binary #3655

merged 1 commit into from
Jan 21, 2021

Conversation

jarkkojs
Copy link
Collaborator

As Python2 has been EOL for over a year already, move on using Python3. The
use of Python2 is likely cause the following buid error in recent versions
of various popular Linux distributions:

[ 67%] Generating lintemp/ScalablePiggy.S
/bin/sh: 1: python: not found
gmake[3]: *** [CMakeFiles/surge-vst3-dll.dir/build.make:140: lintemp/ScalablePiggy.S] Error 127
gmake[2]: *** [CMakeFiles/Makefile2:562: CMakeFiles/surge-vst3-dll.dir/all] Error 2
gmake[1]: *** [CMakeFiles/Makefile2:624: CMakeFiles/Surge-VST3-Packaged.dir/rule] Error 2
gmake: *** [Makefile:332: Surge-VST3-Packaged] Error 2

Do the migration by

  1. Renaming emit-vector-piggy.py as emit-vector-piggy.
  2. Renaming generate-lv2-ttl.py as generate-lv2-ttl.
  3. Giving +x for both.
  4. Using /usr/bin/env to find the location of the interpreter in the
    POSIX standard manner, thus making the whole compilation process more
    distro agnostic.

[*] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/env.html

Signed-off-by: Jarkko Sakkinen jarkko.sakkinen@iki.fi

@jarkkojs
Copy link
Collaborator Author

I've converted from Ableton Live user into Reaper user over last year, moved most of my music production to Linux, a lot thanks to recent developments (mainly PipeWire), and prefer keeping my VST's at my home directory, so using self-compiled Surge is the best bet for my needs :-)

@baconpaul
Copy link
Collaborator

Thanks

halpy to merge this but we are pausing merges for this week while we stabilize 18. Will merge it as soon as we know our merge strategy for either 181 or 19

@jarkkojs
Copy link
Collaborator Author

NP. Merge when you see appropriate. I'm running now self-compiled main branch and obviously have this patch in my fork.

@jarkkojs
Copy link
Collaborator Author

I'll probably rebase this weekly basis up until it is merged as I want to run the latest and greatest...

@baconpaul
Copy link
Collaborator

Yeah depending on our path the latest greatest might get a little rocky but I will merge this early in cycle so you can build close to 18

enjoy!

@jarkkojs
Copy link
Collaborator Author

If I find any other issues the fixes will be visible in this PR (assuming that I'm capable fixing them, otherwise I'll just report an issue). If you don't want to take all at once, git cherry-pick is a great asset,

@mvf
Copy link
Collaborator

mvf commented Jan 19, 2021

Making the python-ness transparent by adding a shebang, removing the .py extension and making it executable is good, but the interpreter should stay python (i.e. "any Python") since the code is compatible with both 2 and 3. In properly configured python-3-only systems, python points to python3.

@jarkkojs
Copy link
Collaborator Author

@mvf, NP I can refine this. Thanks for the feedback.

Off-topic: how can I test run the new flashier skin seen in videos and screenshots?

@jarkkojs
Copy link
Collaborator Author

jarkkojs commented Jan 21, 2021

BTW, In my Debian system python does not point to python3, so should I consider Debian default install is improperly configured? :-) I guess I need to install python-is-python3 package but it was not preinstalled in the default Gnome 3 desktop install. That's where the "design choice" originates from anyway. Fully agree with the logic anyway.

@TheNerdyMusicGuy
Copy link

@jarkk0sakkinen The new skin is in the latest release. Just right click anywhere on the interface and go to Skins -> Royal Surge.

@jarkkojs
Copy link
Collaborator Author

I use self-compiled master mostly because I prefer a local install over deb and also to keep in phase where things are going. I did a compile of the master when I submitted this patch. Was it added after that, or do I do some tricks to get it working? Does it get piggy packed to the binary?

1. Rename emit-vector-piggy.py as emit-vector-piggy.
2. Rename generate-lv2-ttl.py as generate-lv2-ttl.
3. Give +x for both.
4. Use /usr/bin/env to find the location of the interpreter in the
   POSIX standard manner, thus making the whole compilation process more
   distro agnostic.

See: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/env.html

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@iki.fi>
@jarkkojs
Copy link
Collaborator Author

OK, now the commit has been updated (and the commit message has been changed accordingly).

@mvf
Copy link
Collaborator

mvf commented Jan 21, 2021

Sorry about the "properly configured" it's a bit snarky and not very accurate. For the Debian GNOME install I reckon Python was only installed as a dependency and there was no need to install a python symlink. So the system was "properly configured" for running GNOME, but not for running Python scripts off the internet. 😄

Anyhow, looks good now, merging!

@mvf mvf merged commit 18bbe6b into surge-synthesizer:main Jan 21, 2021
@baconpaul
Copy link
Collaborator

baconpaul commented Jan 21, 2021

Uhhh.... you merged this @mvf? I was hoping to wait until after 181. Are we sure it works on all our distros including Rpi
?

@baconpaul
Copy link
Collaborator

@jarkk0sakkinen if you self build you will need to stage the extra content repo. The new skin is in a separate repo which isn’t a submodule. There’s a cmake rule to do it

@baconpaul
Copy link
Collaborator

Oh I see merged with oyhton3 renamed to python. All good

@jarkkojs
Copy link
Collaborator Author

@baconpaul, Thanks, I'll look into it.

@baconpaul
Copy link
Collaborator

Or just clone surge-extra-content into your documents surge folder more easily

@jarkkojs
Copy link
Collaborator Author

Sounds like better plan thank you :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants