Update readme and fix python tests #238

ElectricRCAircraftGuy · 2020-12-23T07:54:21Z

This is a major improvement to the readme, significantly improving build instructions. I also improve general usage instructions, and I port the readme to Github Flavored Markdown, which is more widely-used, widely-understood, and widely-supported. Lastly, I fix two python test warnings, See commit a1d58ae and 03ee1f8 for detailed descriptions.

fixes Native dependency 'fuse3' not found #99
helps with How to build from source in macOS? #121

…(GFM)

…hfs.py

@pytest

``` test/conftest.py:66 sshfs/build/test/conftest.py:66: PytestDeprecationWarning: @pytest.yield_fixture is deprecated. Use @pytest.fixture instead; they are the same. @pytest.yield_fixture(autouse=True) ```

Warning: ``` test/util.py:99 sshfs/build/test/util.py:99: PytestUnknownMarkWarning: Unknown pytest.mark.uses_fuse - is this a typo? You can register custom marks to avoid this warning - for details, see https://docs.pytest.org/en/stable/mark.html return pytest.mark.uses_fuse() ``` References for the fix: 1. https://stackoverflow.com/questions/60806473/pytestunknownmarkwarning-unknown-pytest-mark-xxx-is-this-a-typo/60813297#60813297 1. https://docs.pytest.org/en/stable/mark.html

Changes to be committed: modified: ../README.md new file: ../test/README.md

ElectricRCAircraftGuy · 2020-12-23T08:08:51Z

To see what the new readme looks like, look at my repo here: https://github.com/ElectricRCAircraftGuy/sshfs/tree/update_readme.

ElectricRCAircraftGuy · 2020-12-23T08:09:20Z

Why is Travis CI failing? It might be because it is set to use an old Python 3 version (3.5)? instead of something newer?

Nikratio · 2020-12-23T16:08:45Z

README.md

+### 1. General overview
+
+First, download the latest SSHFS release from https://github.com/libfuse/sshfs/releases. On Linux
+and BSD (as opposed to MacOS), you will also need to install [libfuse][libfuse] 3.1.0 or newer. On


On MacOS, you need to install OSXFUSE I believe.

This is stated just one line below.

Nikratio · 2020-12-23T16:08:58Z

README.md

+MacOS, you need [OSXFUSE][OSXFUSE] instead. Finally, you need the [Glib][Glib] library with
+development headers, which should be available from your operating system's package manager.
+
+To build and install, we recommend you use [Meson][Meson] (version 0.38 or newer) and


I think that's a requirement, not a recommendation.

Nikratio · 2020-12-23T16:12:27Z

README.md

+_Tested 22 Dec. 2020 on Ubuntu 20.04._
+
+1. Download and `cd` into the source code
+    1. To download the latest source code:


The people who install from Git should not need these instructions, so I think this can be safely dropped.

I'm trying to write with the lowest common denominator in mind, meaning even for the beginner developer who may need this help. With your permission, I'd like to leave these details. Just a handful of years ago I needed these instructions myself, and not having many repos with these instructions slowed my progress into making contributions.

That makes sense, thanks for explaining. How about keeping the instructions but moving them into a separate file ("contributor_info.md" or something like that?). That way, the information is available for those who need it without getting in the way of people who don't need it.

👍, moving how to build from git to CONTRIBUTING.md would be good.

Nikratio · 2020-12-23T16:13:19Z

README.md

+    # run the python3 tests in the "test" dir
+    python3 -m pytest test/
+    ```
+    For [sample test output, see here](test/README.md).


Not sure why it's important to have example output? The tests clearly state if they fail or pass....

It took my a lot of effort and time to get the tests to pass, as you can see from my ssh setup instructions, so I was never able to see what was being tested or what was supposed to happen until I had solved many problems and gotten the tests to run. This removes that mystery.

That being said, there's not a ton of value in it. It's just nice to see what's supposed to happen is all.

Nikratio · 2020-12-23T16:13:48Z

README.md

+        ```bash
+        sudo ninja install
+        ```
+    1. "Light" method. This technique simply creates a symlink to the executable in your `~/bin` 


Please drop this. People who want to do something like this should know how to do it without needing instructions.

Is this change blocking? If so, I'll remove it. If not, this info. teaches people who want to do this but don't know how. It took me years to figure this out.

Nikratio · 2020-12-23T16:14:17Z

README.md

+        sudo ninja install
+        ```
+    1. "Light" method. This technique simply creates a symlink to the executable in your `~/bin` 
+       dir so your Linux distro's install still remain's intact and untouched. (For Ubuntu).


This is the case also when using ninja install. Your system's sshfs is in /usr/bin, ninja install into /usr/local/bin.

What's the ninja uninstall command?

I don't know. But I also don't see the connection to what I'm saying...? If you install in /usr/locl, then your Linux distro's install remains intact and untouched.

If you install in /usr/locl, then your Linux distro's install remains intact and untouched.

Agreed. So, if someone wants to go back to using their Linux distro's version of sshfs, how do they do that? The logical answer is: run the ninja uninstall command. That's why I asked "what's the ninja uninstall command?" I'd like to write it down here too.

For the "Alternative method" here, I show: rm ~/bin/sshfs.

A few options to uninstall the ninja install might be:

rm /usr/local/bin/sshfs

Or: modify the path to include /urs/local/bin after /usr/bin instead of before it (probably not recommended), or (best, if such a thing exists): sudo ninja uninstall.

Have you tried running ninja uninstall? If I understand https://mesonbuild.com/Release-notes-for-0-38-0.html correctly, then this should exist and is probably the best approximation to a true uninstall that we have.

Nikratio · 2020-12-23T16:16:07Z

Looks like the pytest version that is being installed isn't compatible with Python 3.5. We'll have to either pin to an older pytest version, or test with a newer Python release.

ElectricRCAircraftGuy · 2020-12-23T16:26:20Z

Looks like the pytest version that is being installed isn't compatible with Python 3.5. We'll have to either pin to an older pytest version, or test with a newer Python release.

Python 3.5 is really old. Let's test with a newer Python release. How do we do that?

ElectricRCAircraftGuy

@Nikratio , I addressed comments. Let me know if these unchanged items are blocking.

ElectricRCAircraftGuy · 2020-12-23T17:17:39Z

README.md

+MacOS, you need [OSXFUSE][OSXFUSE] instead. Finally, you need the [Glib][Glib] library with
+development headers, which should be available from your operating system's package manager.
+
+To build and install, we recommend you use [Meson][Meson] (version 0.38 or newer) and


ElectricRCAircraftGuy · 2020-12-23T17:19:39Z

README.md

+    # run the python3 tests in the "test" dir
+    python3 -m pytest test/
+    ```
+    For [sample test output, see here](test/README.md).


That being said, there's not a ton of value in it. It's just nice to see what's supposed to happen is all.

ElectricRCAircraftGuy · 2020-12-27T20:33:45Z

@Nikratio , which items are still blocking? I'll address any blocking issues to ensure you are okay merging it.

I can add your proposed contributor_info.md, or similar, file, for some of the details.

I'd like to have Travis CI use a later Python version. I'm not entirely sure how to do that. Some ideas include:

Update this file to use bionic (Ubuntu 18.04) instead of xenial (Ubuntu 16.04). I think that'll bring in a newer Python version automatically. Or:
Update this file to manually install a later version of Python.

I prefer option 1 if that works as I think it will. Is there any reason not to update to 18.04, bionic?

So, what do I need to do to get this PR merged? Which items are blocking vs recommended but not blocking?

Nikratio · 2020-12-28T15:53:44Z

I think updating the CI to bionic is fine.

If you can move the alternative installation options to a separate file, I'd be happy to merge this.

Nikratio · 2021-02-28T09:44:04Z

Any update on this?

ElectricRCAircraftGuy · 2021-03-01T06:59:14Z

@Nikratio , I haven't had time to look at this again yet. Still on my todo list. If you'd like to make the fixes and merge, feel free to follow the github "command-line" instructions you should be able to see here by the merge button, then I believe you can check out my branch, make any changes yourself you see fit, and push to the master branch.

If you are unable to take it from here and make the fixes, I'll get back to it when able. Note that so long as you keep my commits intact and just add new commits on top of my commits to fix my commits, I will still show up as a contributor, so you making corrections to my PR here won't take that away.

Nikratio · 2021-04-28T08:17:21Z

Are you still interested in working on this? If not, I'll close the pull request in a few days.

ElectricRCAircraftGuy · 2021-04-28T12:12:09Z

@Nikratio , Would you be able to manually check out my branch, add the changes you'd like, and manually merge them to the main branch? If so, you get the fixes you want and I get credit for the parts of the PR you kept.

ElectricRCAircraftGuy added 14 commits December 22, 2020 20:28

rename README.rst --> README.md

1631a9f

Convert README.md from Restructured Text to Github Flavored Markdown …

006b1fa

…(GFM)

README.md: fix up all parts except for "Installation" section

dc950df

Update README

3a7bc12

README.md: fix links

f594fc2

README.md: wip: rewriting build and install instructions

ca7d695

Finish build and install notes in README.md; fix error msg in test_ss…

865c92f

…hfs.py

README.md: update keygen notes & version output

954fc37

Update README

bb51c78

Fix deprecated warning in conftest.py

a1d58ae

``` test/conftest.py:66 sshfs/build/test/conftest.py:66: PytestDeprecationWarning: @pytest.yield_fixture is deprecated. Use @pytest.fixture instead; they are the same. @pytest.yield_fixture(autouse=True) ```

Add sample test output

56b6763

Changes to be committed: modified: ../README.md new file: ../test/README.md

Update test/README.md

a91d12f

Update link in test/README.md

ab21324

Nikratio requested changes Dec 23, 2020

View reviewed changes

README.md: address a few comments; do a few improvements

f923cd0

ElectricRCAircraftGuy commented Dec 23, 2020

View reviewed changes

ElectricRCAircraftGuy requested a review from Nikratio December 23, 2020 17:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update readme and fix python tests #238

Update readme and fix python tests #238

ElectricRCAircraftGuy commented Dec 23, 2020 •

edited

Loading

ElectricRCAircraftGuy commented Dec 23, 2020

ElectricRCAircraftGuy commented Dec 23, 2020

Nikratio Dec 23, 2020

ElectricRCAircraftGuy Dec 23, 2020

Nikratio Dec 23, 2020

ElectricRCAircraftGuy Dec 23, 2020

Nikratio Dec 23, 2020

ElectricRCAircraftGuy Dec 23, 2020

Nikratio Dec 27, 2020

kalvdans Apr 15, 2024

Nikratio Dec 23, 2020

ElectricRCAircraftGuy Dec 23, 2020

ElectricRCAircraftGuy Dec 23, 2020

Nikratio Dec 23, 2020

ElectricRCAircraftGuy Dec 23, 2020

Nikratio Dec 23, 2020

ElectricRCAircraftGuy Dec 23, 2020

Nikratio Dec 27, 2020 •

edited

Loading

ElectricRCAircraftGuy Dec 27, 2020

Nikratio Dec 28, 2020

Nikratio commented Dec 23, 2020

ElectricRCAircraftGuy commented Dec 23, 2020

ElectricRCAircraftGuy left a comment

ElectricRCAircraftGuy Dec 23, 2020

ElectricRCAircraftGuy Dec 23, 2020

ElectricRCAircraftGuy commented Dec 27, 2020 •

edited

Loading

Nikratio commented Dec 28, 2020

Nikratio commented Feb 28, 2021

ElectricRCAircraftGuy commented Mar 1, 2021

Nikratio commented Apr 28, 2021

ElectricRCAircraftGuy commented Apr 28, 2021

Update readme and fix python tests #238

Are you sure you want to change the base?

Update readme and fix python tests #238

Conversation

ElectricRCAircraftGuy commented Dec 23, 2020 • edited Loading

ElectricRCAircraftGuy commented Dec 23, 2020

ElectricRCAircraftGuy commented Dec 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nikratio Dec 27, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nikratio commented Dec 23, 2020

ElectricRCAircraftGuy commented Dec 23, 2020

ElectricRCAircraftGuy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ElectricRCAircraftGuy commented Dec 27, 2020 • edited Loading

Nikratio commented Dec 28, 2020

Nikratio commented Feb 28, 2021

ElectricRCAircraftGuy commented Mar 1, 2021

Nikratio commented Apr 28, 2021

ElectricRCAircraftGuy commented Apr 28, 2021

ElectricRCAircraftGuy commented Dec 23, 2020 •

edited

Loading

Nikratio Dec 27, 2020 •

edited

Loading

ElectricRCAircraftGuy commented Dec 27, 2020 •

edited

Loading