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

Windows setup on Local Environment #5321

Conversation

sharon-odhiambo
Copy link
Contributor

@sharon-odhiambo sharon-odhiambo commented Mar 22, 2023

NOTE: Please review the pull request process before opening your first PR: https://github.com/WikiEducationFoundation/WikiEduDashboard/blob/master/CONTRIBUTING.md#pull-request-process

What this PR does

  • This PR fixes the bugs on windows setup script and allows Windows users to easily run the script on their local environment and access the dashboard locally.
  • It also sorts the js commands in build preparation that do not take care of the Windows commands.
  • With mysql server configuration done properly, the script successfully installs all necessary gems and dependencies, performs database configurations and migrations.
  • This PR also automates the debian script for WSL users
  • It solves this issue

Screenshots

Before:
Screenshot (3)

After:
Screenshot (1)
Screenshot (2)

Open questions and concerns

Kind;y ensure that when using Windows there are a number of commands in the prepare.js that are Unix-based and might not run in Windows. I've updated some but when you run into any error when building you can check the error log and update the commands with conditionals based on the system.

@ragesoss
Copy link
Member

This should not include any changes to schema.rb. It probably also should not include changes to the Gemfile — if those are necessary, perhaps they can be documented in the setup instrutions? — although if we can get the build passing with the platform-specific entries, it might be fine.

@sharon-odhiambo
Copy link
Contributor Author

sharon-odhiambo commented Mar 23, 2023

Hi @ragesoss , can I add the db schema to the gitignore?. For the Gemfile I'm wondering whether I can add it in the setup instructions for Windows users to comment out stackprof in their local and not push that change upstream?

In the meantime I can check the ones that will pass the check.

@ragesoss
Copy link
Member

@sharon-odhiambo no, you can't add schema.rb to the gitignore... it should just only change when there are new migrations, usually. the schema.rb file is automatically generated whenever any migrations are run, but it reflects details that are specific to the system it is run on (such as database settings that are not configured by the app itself), so running the existing migrations on different systems results in minor differences in the schema.rb files. We usually just exclude these from commits; you can use git stash after running migrations to reset the changes to it.

The instruction changes you suggest for stackprof sound reasonable.

@ragesoss
Copy link
Member

This should include no changes to schema.rb.

I'm okay with the change to the Gemfile itself (relaxing the Ruby verison), but I'd also prefer not to introduce any changes to Gemfile.lock here, since I'm not entirely sure how it would interact with production.

@sharon-odhiambo
Copy link
Contributor Author

Hi @ragesoss . Sure lemme incorporate this. I will update the Windows specific changes to the Gemfile in the detailed instructions.

@sharon-odhiambo
Copy link
Contributor Author

Hi @ragesoss . Incorporated the feedback.

Copy link
Member

@ragesoss ragesoss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I want to test this on a fresh Windows system, but that will be a time-consuming thing for me to prepare... my Windows devices might not be in usable states at the moment. I'm adding it to my todo list to test this out when I have a chance to do so.

@@ -117,6 +117,8 @@ else
fi

printf '[*] Installing Gems... \n'
output_line "sudo apt install libmysqlclient-dev" && print_success "${CLEAR_LINE}[+] Gems installed\n"
output_line "gem install mysql2" && print_success "${CLEAR_LINE}[+] Gems installed\n"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be necessary to do separately, as mysql2 gem is part of the bundle specified in the Gemfile.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ragesoss, thank you for the feedback. Mysql2 gem installation was a bit problematic with the Gemfile in debian when mysqlclient is not installed so including it was helpful. So should I remove both line 120 and 121 or just 121?

setup.py Outdated
@@ -36,7 +36,10 @@ def osx_setup():
subprocess.run("sudo chmod 775 setup/macOS-setup.sh && setup/macOS-setup.sh",
shell=True, check=True)

if platform.platform().lower().find('ubuntu') != -1 \
if "microsoft" in platform.uname().release.lower():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the computer is running Windows but does not have WSL (either because it hasn't been installed or enabled, or because it's running an older version of Windows that does not support WSL)? I would guess that it might still have microsoft in the platform name that Python sees.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ragesoss, let me check this. Just a clarification, I'm I supposed to confirm that it doesn't run the debian script when the machine has only Python without WSL?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure. It should either work no matter what, or it should fail gracefully by detecting that the system isn't compatible with the scripts and exiting with an appropriate message.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay then. In that case can I check if WSL is enabled or installed and run the debian script, else I can run the windows script? I included in the documentation that WSL is the recommended option but in case the version doesn't support WSL they can fallback to Windows script. Would this be okay?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you know that the debian script works on WSL, then that makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it does. Okay lemme go ahead and do that. Thank you.

@sharon-odhiambo
Copy link
Contributor Author

sharon-odhiambo commented Apr 3, 2023

Hi @ragesoss , kindly ,let me know if this is okay now.

@ragesoss
Copy link
Member

ragesoss commented Apr 3, 2023

Thanks! I'll test it on a clean Windows device when I get a chance to.

@sharon-odhiambo
Copy link
Contributor Author

Okay. Thank you.

@sharon-odhiambo
Copy link
Contributor Author

Hi @ragesoss , I have a further question on this. Before I automated this script and fixed the bug on Windows script, it would not run at all. However at the moment, If you run the setup script from Ubuntu terminal for WSL users, it runs the debian script. If you run it from windows bash terminal, it runs the Windows script even if the version has WSL. Is that the desired behavior?

@sharon-odhiambo
Copy link
Contributor Author

Hi @ragesoss . Another one again 😆 . Did you manage to test this?

@ragesoss
Copy link
Member

I reset my Windows laptop in preparation, but haven't tested it yet.

@ragesoss
Copy link
Member

ragesoss commented May 1, 2023

Okay, I'm finally working on this... I started from the WSL setup doc, https://github.com/WikiEducationFoundation/WikiEduDashboard/blob/master/docs/ubuntu_wsl_setup.md — and I'm realizing that I don't know exactly how I ought to integrate this with your script updates... the script is not designed to install Ruby, and while the WSL setup doc says to use RVM from within an ubuntu terminal, in the general setup doc is still says to use RubyInstaller. Which path did you take? I think the RVM method from the current WSL setup doc will be the more reliable one, but I'm not certain.

@sharon-odhiambo
Copy link
Contributor Author

So for WSL option I think I used RVM, but for the Windows users who can't completely use WSL I used RubyInstaller. Maybe I should update that in the documentation.

@ragesoss
Copy link
Member

ragesoss commented May 1, 2023

Cool. I think we should switch to recommending WSL and RVM as the default method, as RubyInstaller is never going to be as reliable in terms of version availability, and WSL seems pretty solid as a way of running a dev environment now.

@sharon-odhiambo
Copy link
Contributor Author

Yeah sure, WSLis quite effective at the moment. Updated the documentation.

@ragesoss
Copy link
Member

ragesoss commented May 1, 2023

Okay! I've install Ruby on WSL on my test machine, downloaded the git repo, checked out this branch, and attempted to run python3 setup.py. However, the script immediately exits with the message "Sorry! Your operating is not supported by this script".

@ragesoss
Copy link
Member

ragesoss commented May 1, 2023

I ran the platform command from the setup script (from within the Ubuntu terminal, as I think will be the right way to do it for WSL) and it gives "Linux-5.10.16.3-microsoft-standard-WSL2-x86-with-glibc2.3.5". I think checking for wsl2 should be a good way to check for the windows branch of the script... but now that I've come this far, I think we might be able to just use the debian script from within WSL, rather than running a separate Windows one. I'm trying it out.

@ragesoss
Copy link
Member

ragesoss commented May 1, 2023

It didn't successfully install mysql / mariadb from within Ubuntu... or at least, it didn't start it up successfully in a way that worked from the script to do the database setup. But the rest of it seems to have worked so far...

@ragesoss
Copy link
Member

ragesoss commented May 1, 2023

This appears to be because mariadb didn't start automatically. After starting it with the command sudo /etc/init.d/mariadb start (adapted from
microsoft/WSL#745 (comment)), mysql was working. I re-ran the script but it still failed with the database operations because the wiki@localhost user couldn't connect. That's just because of a flaw in the deb script that relies on the existence of setup/database.yml to determine whether to create a mysql user, so I was able to delete that file and run it again, and it worked... so it ought to work the first time, if we update the script to somehow ensure that the database gets started after installation on wsl2.

@ragesoss
Copy link
Member

ragesoss commented May 2, 2023

After successfully getting through the setup process on a fresh Windows 10 installation using the WSL route, my thinking is that our best option is probably to start saying that WSL is the only supported method of setting it up on Windows, and that we should remove the documentation and scripts that are for non-WSL Windows setup.

Someone experienced with Windows and running Ruby without WSL is probably also going to be able to figure out the best current options for getting through the setup process based on the normal manual install instructions, but fewer and fewer new developers are going to be trying to contribute to this project with a Windows device that could run the project without WSL but isn't compatible with WSL. There are already too few who have come through the non-WSL setup path to the point where we haven't been able to maintain a working Windows setup script.

I appreciate your thoughtful work on this @sharon-odhiambo . Even though this isn't going to be merged, it was really helpful for getting the setting system to a better state for Windows users.

@ragesoss ragesoss closed this May 2, 2023
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

2 participants