getsolus / 3rd-party

Extra 3rd party packages
21 stars 23 forks source link

Upgraded Sublime Text 3 to Sublime text 4 #55

Closed Abhinav1217 closed 1 year ago

Abhinav1217 commented 3 years ago

Upgraded to sublime text 4

Possible issue regarding this upgrade.

I wanted to have separate package for st4, but that approach has few main issues. The license changes applies to current st3 users too. And while st4-dev could co-exist along with st3, release build does not. Their forum is currently getting a lot of compatibility complaints after upgrade. Commands like subl sublime_text will also clash.

Moreover, having no version in binaries and path is something that they have now adopted, for example their install guide which previously said /opt/sublime_text_3/* now says /opt/sublime_text without the _4 part. Same for their binaries, and config locations.

Therefore this one time hardship for upgrade in exchange to clear upgrade path in future feels worth it. However if solus team can think of any workaround where user can seamlessly upgrade the package, please let me know.

Abhinav1217 commented 3 years ago

Any feedback on this?

Abhinav1217 commented 3 years ago

Has anyone taken any look at this?

Abhinav1217 commented 3 years ago

@Staudey Is there not interest in sublime text any more? I don't want to host eopkg as my personal fork?

Staudey commented 3 years ago

@Abhinav1217 first of all, thank you for your contribution! I just haven't found the time yet to take a close look at it, seeing as a lot has been going on at work, even today. Also I probably will have to get the input of a Core Team member when it comes to a possible package renaming.

Abhinav1217 commented 3 years ago

Thanks for clarification. I knew renaming would be somewhat an issue. I wanted to have a separate package for st4 initially, But new licensing terms made more sense for one time hassle of renamed package conflict because from here there should be clear upgrade path in future. But since there is no way to uninstall 3rd party package from software center, I know this would be big issue.

I tried having a separate sublime_text_4 package, but even that was conflicting with st3 on icons, so I figured renaming the st3 package itself would make more sense. But if you want, I can submit a PR for "separate package st4" instead of renamed "st4"

Abhinav1217 commented 3 years ago

@Staudey I have updated this pull request to Sublime Text 4 to 4113 Released on 14-July-2021.

Is there still no Update on if this would be merged or not? I can keep maintaining it but if there is no interest in merging it, then whats the point.

Abhinav1217 commented 3 years ago

I have already updated my fork with Build 4113, but you are commenting on 4107, So I guess that update didn't automatically reflected here. I thought I checked it.

Can you guide me on proper commands for squashing on an open PR? I don't think I ever did squashing before. Or would it be better if I close this and open a new PR after doing all the modifications you suggested here.

Abhinav1217 commented 3 years ago

Conflict resolution is working properly. Is there a link to docs I could have learned this from, I did spent few hours looking for a solution to this.

I have an weird issue. I switched from hard-coded version to get.srcVERSION() command. Now after installing the package, subl command is not working, which is working out-of-the-box if I install it again after switching to versioned line. So I figured I could add another dosym command.

As per docs all this commands do is to create symbolic links, but I get this error

DEBUG: Removing special python file: /var/eopkg/sublime-text-4113-11/install/opt/sublime_text/Lib/python3/certifi/__pycache__/__init__.cpython-38.opt-1.pyc
DEBUG: Removing special python file: /var/eopkg/sublime-text-4113-11/install/opt/sublime_text/Lib/python3/certifi/__pycache__/core.cpython-38.opt-1.pyc
DEBUG: Removing empty dir: /var/eopkg/sublime-text-4113-11/install/opt/sublime_text/Lib/python3/certifi/__pycache__
There are abandoned files under the install dir (/var/eopkg/sublime-text-4113-11/install):
    - /usr/bin/subl

I get this (or similar) error in any combination of command I try, even tried to execute shell command using shelltools.system("ln -s ........

Any pointers? I basically want to have two symbolic links ( /usr/bin/sublime_text and /usr/bin/subl ) pointing to the executable.

Staudey commented 3 years ago

I switched from hard-coded version to get.srcVERSION() command. Now after installing the package, subl command is not working, which is working out-of-the-box if I install it again after switching to versioned line. So I figured I could add another dosym command.

I just saw that I had a typo in my version of the command. It's shelltools.system("tar xf sublime-text_build-%s_x64.tar.xz" % (get.srcVERSION())) with an underscore before the "%s" not a hyphen. In case you copied it directly (but I guess you would've gotten an error then).

So pisitools.dosym("/opt/sublime_text/sublime_text", "/usr/bin/subl") doesn't work?

Abhinav1217 commented 3 years ago

I just saw that I had a typo in my version of the command. It's shelltools.system("tar xf sublime-text_build-%s_x64.tar.xz" % (get.srcVERSION())) with an underscore before the "%s" not a hyphen. In case you copied it directly (but I guess you would've gotten an error then).

No I saw that and corrected it. The eopkg is building properly except for subl integration.

So pisitools.dosym("/opt/sublime_text/sublime_text", "/usr/bin/subl") doesn't work?

No, It gives me the error

There are abandoned files under the install dir (/var/eopkg/sublime-text-4113-11/install):
    - /usr/bin/subl

I tried a few combinations of it.

subl command is quite important because of muscle memory of us sublime text users, A lot of us even have script usage of that command even though sublime_text is recommended instead of shortcut. I know it is easily solved by end users using alias but it is confusing why dosym is not working. Is there some undocumented difference between original pisitools and the iKey fork for eopkg?

Abhinav1217 commented 3 years ago

I tried on a clean install of solus in virtual machine, dosym still gives same error. Any pointers?

Staudey commented 3 years ago

I've played around with it for a bit and am just as confused as you are. Couldn't get it to work. Maybe when I take a fresh look tomorrow something will come to mind.

Abhinav1217 commented 3 years ago

@Staudey Until you figure out dosym solution, should I submit the installer package? Other than adding another symlink, everything is working and has been updated as per your comments. I guess for the time being, we can let the users manually set the alias. Once you figure out the issue, I can update the package to incorporate that.

I would also need help from you on proper steps for squashing the commits in PR. I don't want to mess things up.

JoshStrobl commented 3 years ago

Unless the transition is flawless between 3 to 4, I have no intentions on merging this. We are also not including any supplemental packages, so this must be a replaces as opposed to a conflicts.

Abhinav1217 commented 3 years ago

@JoshStrobl Thanks to <Conflicts> suggestion (and other tips) provided by @Staudey , The upgrade is seamless.

There are no supplemental package. Like I mentioned before, They have changed the licensing terms for current ST-3 too. So unlike in past when ST-2 and ST-3 should be treated as a separate package because they were given license based on their major version, Now they all fall under a Time based license. So it is just Sublime text from now on.

I am not sure what you mean by supplemental package? I know that you have been trying to phase out 3rd party packages in favor or snap, but sublime text doesn't provide any official snap (or flatpak or appimage). This PR will be just a package upgrade not a new addition.

Staudey commented 3 years ago

so this must be a replaces as opposed to a conflicts.

Right, for some reason I thought that that didn't exist in old-school eopkg ^^

@Abhinav1217 Please add the following instead of the section:

        <Replaces>
            <Package>sublime-text-3</Package>
        </Replaces>

Btw I think you haven't yet created pull request for the necessary changes in help-center-docs and solus-sc. Should I prepare those changes in your stead?

Abhinav1217 commented 3 years ago

@Staudey

I will add the <Replace> section to the repo.

Other than the mysterious issue of not being able to use dosym multiple times, everything is working and thanks to <Conflict> section, upgrade from older ST3 to ST4 is smooth. Once someone figure out a fix for it, I can submit an update for it. I feel that for now, we should update the package and let the end user create their own alias on their system.

Btw I think you haven't yet created pull request for the necessary changes in help-center-docs and solus-sc. Should I prepare those changes in your stead?

Yes please, I do not feel confident to mess with core system. I am happy with the small stuff like managing the packages.

I haven't heard back from @JoshStrobl on my response to his comment, Can I submit the PR for merging (after updating it with ) ?

Staudey commented 3 years ago

@Abhinav1217 Yes, please update this PR with the requested changes (<Replaces> instead of <Conflicts>, updating the actions.py to be version-independent, shortening the <Update release....> section to only show a single release for the introduction of Sublime Text 4). After that I will check with Josh if he is comfortable merging it.

I've prepared the necessary changes for solus-sc and help-center-docs in the meantime:

https://github.com/getsolus/solus-sc/pull/103 https://github.com/getsolus/help-center-docs/pull/294

edit: and I will think about the symlink issue again, maybe we can figure it out before the merge, otherwise it's not a huge deal if we do it at some point later. Was this even working with the current version?

Abhinav1217 commented 3 years ago

Was this even working with the current version?

As far as I remember, I never have to add any alias or symlink before this. I will spend a day testing some combinations to make sure.

I will send a PR with all modifications by Saturday.

Staudey commented 2 years ago

Hello @Abhinav1217 Just wanted to ask if you're still planning to update this PR?

Abhinav1217 commented 2 years ago

I totally forgot about this. I am out of town currently without my laptop. I will be back on Sunday. I have commits ready, I just forgot to push it to github.

Staudey commented 2 years ago

No worries, and no rush! I was just wondering if you were even interest at all, because if not I would've copied your code and implemented those changes myself.

glow12121 commented 2 years ago

I have commits ready, I just forgot to push it to github.

That would be very nice, thanks.

Abhinav1217 commented 2 years ago

I am sorry for all the delays. I got fixated to try to add subl command out of the box, and then frustration quit. Today, I had to configure a new install of solus on an office computer, only to realize that I haven't pushed the update from my system to github repo.

The package is working fine, and there is no conflict. If someone still has ST-3 installed, The upgrade should be seamless. Please include this to the software center.

I will try to make some adjustment to my work-life balance so that next time when ST is updated, I will update pspec.xml quickly, without going through months of delay like this time.

Staudey commented 2 years ago

@Abhinav1217 Sorry, now it was me again who was slow to react (busy with life, but fortunately in a good way). I will see how quickly we can get the updated Help Center documentation and Software Center PR merged and published, and then I (or someone else) will merge this pull request, to finally get this done. Thanks once again for your contribution! Only took us about a year to get here :grin:

Abhinav1217 commented 2 years ago

Only took us about a year to get here grin

Oh my god, I just had to look, May 27, 2021, when I opened this PR. 😲

I knew I was getting lax, but this just makes me realize I need to work a lot on sorting my life. Maybe I should look into therapy again.

Also, I am still willing to maintain this plugin. Since the major work has been sorted out, When next update is released, It won't take so long.

Staudey commented 2 years ago

To be fair, during the first couple months the inertia was all on my side. That's probably why we lost steam ^^

glow12121 commented 2 years ago

Sublime Text update would still be much welcome.

Abhinav1217 commented 2 years ago

The PR is working, and when it will merged, it should seamlessly upgrade st3 to st4. I don't think there is any blocker since https://github.com/getsolus/solus-sc/pull/103 and https://github.com/getsolus/help-center-docs/pull/294 is already merged.

All that remains is an lgtm from one of the project maintainers for this to work.

Staudey commented 2 years ago

I just have to coordinate the merging and deploying of everything with DataDrake (since every part should happen at roughly the same time, so we don't arrive at an inconsistent state between this repo, Help Center, and Software Center). But that's the only remaining difficulty, yes.

glow12121 commented 2 years ago

@DataDrake

Staudey commented 2 years ago

Please don't just ping people. I can talk with DataDrake myself ^^

glow12121 commented 2 years ago

I won't. Didn't know that.

glow12121 commented 2 years ago

Nothing has changed.

glow12121 commented 2 years ago

Hopefully this update will happen after all.

Abhinav1217 commented 2 years ago

@Staudey any progress in https://github.com/getsolus/3rd-party/pull/55#issuecomment-1165630169 ?

Staudey commented 2 years ago

Sorry, no news yet. The Help Center update hasn't happened, so there was no chance to pull these changes in.

Abhinav1217 commented 2 years ago

You mean This one? https://github.com/getsolus/help-center-docs/pull/294 . Is there any other blocker?

The conflict that Josh suggested has already fixed mentioned in comment https://github.com/getsolus/3rd-party/pull/55#issuecomment-897596027 So there should be no big blocker unless I am missing something. The diff in help-docs are just URL changes.

Solus-SC https://github.com/getsolus/solus-sc/pull/103 has been merged, Help center is important for those who use command line instead of Solus-SC which like me should be quite a number of users.

Technically, as it turns out, The Solus-SC should now broken because this PR is still not merged. Here is what I mean:

Since https://github.com/getsolus/solus-sc/pull/103 is already merged as confirmed by the screenshot, means the URL should be pointing to the updated URL as can be seen in diff Which is not yet merged.

Screenshot from 2022-08-13 21-51-06

Staudey commented 2 years ago

Yes, that PR. The conflict that Josh talked about was actually something inside the help-center-docs repository itself, but I already fixed that back then. It just needs to be merged, and, crucially, deployed to the website. While the solus-sc PR has been merged, it hasn't yet been added to our solus-sc package (you can start the installation from the Software Center on Solus and see that it still says "sublime-text-3"). It's just a matter from going live with everything at the same time. Unfortunately in the case of the website that isn't as easy as simply merging the PR, and apparently quite a bit more complicated behind the scenes (which is why DataDrake is currently working on a rewrite of the Help Center to simplify the process). For the solus-sc package it's only a matter of minutes though (and then of it migrating from Unstable to the Stable repository, but we can cherry-pick it when the day comes).

Staudey commented 1 year ago

So, after thinking about this once again I've decided to merge it, but will then restructure it a bit, so the package is still named "sublime-text-3" internally. This enables us to update without immediately requiring all the related work in solus-sc and the Help Center. When those are ready, we can quickly switch back to the unversioned package name. This way we don't have to wait even longer for an updated package ^^

Not the most elegant solution, but at this point I'm okay with that.

Thanks again for the work and patience @Abhinav1217!

Abhinav1217 commented 1 year ago

Finally. But please see that relevant updates are quickly sorted as it will be confusing to see a package named sublime-text-3 to install ST4, or create confusion to those searching for a newer version of ST unaware that it is available despite wrong package name.