PiSCSI / piscsi

PiSCSI allows a Raspberry Pi to function as emulated SCSI devices (hard disk, CD-ROM, and others) for vintage SCSI-based computers and devices. This is a fork of the RaSCSI project by GIMONS.
https://piscsi.org
BSD 3-Clause "New" or "Revised" License
537 stars 82 forks source link

Rename RaSCSI to PiSCSI #986

Closed akuker closed 1 year ago

akuker commented 1 year ago

We have been asked by the original developer of RaSCSI to rename this project to something else. The reasoning being that one of the most valuable features to the Japanese Sharp X68000 community was SASI support. Since this project no longer supports SASI, it is not really true to the original "RaSCSI".

rdmark commented 1 year ago

@uweseimet Do you have any ongoing dev branches that would be disrupted by executing on this rebranding? We're trying to nail down the right timing since the codebase wide search and replace will make other dev work difficult for a short while.

uweseimet commented 1 year ago

@rdmark No, there is nothing pending on my side. Just go ahead.

rdmark commented 1 year ago

Thanks for confirming.

uweseimet commented 1 year ago

@rdmark @akuker Can you outline the scope of the re-branding as far as the C++ code is concerned? In theory one could go as far as renaming files, folders and executables, but I doubt that this is a good idea or required, at least not initially.

In addition to the banners there are pieces of code with a user-visible reference to RaSCSI:

I don't think there are more code locations like that.

akuker commented 1 year ago

On hold for now.... the community was not happy about the new name...

akuker commented 1 year ago

So, the new, new "official" name is PiSCSI. I think we can start planning this now.

Proposed executable changes:

We should have easyinstall move the old files to a temp location.

Do we want to have symbolic links so that rascsi points to piscsi? Probably not necessary for people using the web interface.

uweseimet commented 1 year ago

I don't think that symbolic links are needed. Better to get used to the new names soon ;-).

rdmark commented 1 year ago

A first stab at this... https://github.com/akuker/RASCSI/pull/1016 Testing in progress.

rdmark commented 1 year ago

@akuker Are you able to rename the Sonar project key, or will you have to create a new one from scratch?

uweseimet commented 1 year ago

@rdmark @akuker Note that the magic string "RASCSI" used in the socket communcation must not be renamed. Otherwise the interface contract is broken. You can ensure that it is not broken by running the old rasctl against the new piscsi or the new scsictl against the old rascsi.

Renaming the interface itself from "rascsi_interface" to "piscsi_interface" does not cause issues, i.e. protobuf is lenient here. That's most likely because the interface name is not relevant, but only what's "inside" the interface matters. I have already verified that reverting the magic string fixes the interoperability issues.

The shutdown mode "rascsi" also needs be supported, at least for some time, in order to stay backwards compatible. "piscsi" can be added as an alternative, though. But one could just stick to "rascsi" only here. Regarding the magic string one can also support both "RASCSI" and "PISCSI", even though I again suggest to keep "RASCSI" here. Nobdoy can complain about the string "rascsi" being used in a technical context inside the sources.

If you agree I can apply the required changes to the existing re-branding branch.

akuker commented 1 year ago

I agree. We don't need to completely purge "RaSCSI".

In fact, we should probably outline in the documentation (manpage?) that PiSCSI can not be used at the same time as RaSCSI

akuker commented 1 year ago

@akuker Are you able to rename the Sonar project key, or will you have to create a new one from scratch?

Done! I renamed the project key, so it should match your PR

uweseimet commented 1 year ago

@akuker Nobody reads manpages ;-). IMO the message currently displayed when launching piscsi twice is sufficient, but it should probably say

Error: Port 6868 is in use, is rascsi or piscsi already running?

instead of

Error: Port 6868 is in use, is piscsi already running?
akuker commented 1 year ago

I like it :)

akuker commented 1 year ago

I just did a little testing, and the "transfer ownership" function looks to be seamless. Github will automatically re-direct to the new repo name. So, I'm going to take the plunge and re-name/move the repo. Hopefully this doesn't blow up! :)

rdmark commented 1 year ago

@uweseimet I agree with maintaining backwards compatibility / interoperability for the foreseeable future. It would be advisable to leave code comments in these cases to communicate to future code readers/contributors why certain internal names are different.

Please go ahead and push your changes to the rebrand branch. I'm done rebasing against develop now.

uweseimet commented 1 year ago

@rdmark Done and tested.

I also fixed one or two SonarCloud issues in the new tests ... I wonder why new code is still not C++ but rather C-like. Using C++ would reduce the number of SonarCloud issues and would avoid other potential problems.

Note that I did not change the "is ... already running" messages mentioned above.

uweseimet commented 1 year ago

Can I please get access to the new SonarCloud project? Currently I seem to lack permissions to see it.

rdmark commented 1 year ago

Can I please get access to the new SonarCloud project? Currently I seem to lack permissions to see it.

@uweseimet That's odd, the project is Public so anyone should at least be able to see it. How about now? https://sonarcloud.io/project/overview?id=akuker_PISCSI

rdmark commented 1 year ago

Note that I did not change the "is ... already running" messages mentioned above.

I pushed a change for this. It makes for a more helpful message potentially.

This should be 99% ready to go now, but I found one instance where the unit test code still referenced "RasUtil". I have a local change and am running the unit tests locally to test before pushing.

uweseimet commented 1 year ago

@rdmark Your SonarCloud link works, thank you. I was using the search functionality of SonarCloud before, but it did not find the project.

rdmark commented 1 year ago

The rebrand PR has been merged!

Now, the next task is to add logic to installation scripts to clean up and rename anything "rascsi" before installing, since 1) the two cannot coexist and 2) users expect to have continuity with their user data through the rebranding.

It should be easy enough to add some checks to easyinstall, but I wonder if want the Makefile to check for /etc/systemd/system/rascsi.service before installing piscsi.service next to it?

uweseimet commented 1 year ago

@rdmark Tasks like this should not be part of a Makefile, but of an external script. This might be a new script or easyinstall. Since root permissions are required (I assume), easyinstall appears to be the best candidate in my opinion.

rdmark commented 1 year ago

Data migration introduced to the easyinstall script.

@akuker I believe the only thing left to do is to introduce the new logo, and perhaps finish the wiki editing. Anything else?

rdmark commented 1 year ago

Made a new ticket for the logo so that we can close this one out https://github.com/PiSCSI/piscsi/issues/1024