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

FreeBSD support #1616

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

FreeBSD support #1616

wants to merge 1 commit into from

Conversation

ascl00
Copy link

@ascl00 ascl00 commented Jan 19, 2020

Some changes to make things work a little smoother on FreeBSD (well, FreeNAS technically). _list_disks_gpart() is incomplete, if I find time I'll flesh it out. The changes should make no difference on Linux.

@zajc3w
Copy link

zajc3w commented Mar 15, 2020

Thanks ascl00, hpefully it will be more stable on FreeBSD

@@ -29,7 +29,9 @@ filename="$4"

uri="/_relay_event/?_username=$username&event=$event&motion_camera_id=$motion_camera_id"
data="{\"filename\": \"$filename\"}"
signature=$(echo -n "POST:$uri:$data:$password" | sha1sum | cut -d ' ' -f 1)
# allow for sha1 usage as well as sha1sum
sha1=$(which sha1sum sha1)
Copy link
Member

@MichaIng MichaIng Mar 12, 2022

Choose a reason for hiding this comment

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

Problematic if both commands exist. Also, please use command -v:

Suggested change
sha1=$(which sha1sum sha1)
sha1=$(command -v sha1sum sha1 | head -1)

Comment on lines +37 to +53
# check if it exists before calling to avoid log spam
rc = subprocess.call(['which', 'lsb_release'])

if rc == 0:
try:
output = subprocess.check_output('lsb_release -sri', shell=True)
lines = output.strip().split()
name, version = lines
if version.lower() == 'rolling':
version = ''

return name, version
return name, version

except:
return _get_os_version_uname()
except:
return _get_os_version_uname()
else:
return _get_os_version_uname()
Copy link
Member

Choose a reason for hiding this comment

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

Wrong indentation? Simpler may be:

    # check if it exists before calling to avoid log spam
    if subprocess.call(['which', 'lsb_release']):
        try:
            output = subprocess.check_output('lsb_release -sri', shell=True)
            lines = output.strip().split()
            name, version = lines
            if version.lower() == 'rolling':
                version = ''

            return name, version

        except:
            pass

    return _get_os_version_uname()

Copy link
Member

@cclauss cclauss Mar 12, 2022

Choose a reason for hiding this comment

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

Can we please lose the bare except as discussed in PEP8 and
https://realpython.com/the-most-diabolical-python-antipattern

Copy link
Member

Choose a reason for hiding this comment

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

I also see annotations on this in CodeFactor. What is the exception type in case of a failing subprocess.check_output?

Copy link
Member

@MichaIng MichaIng Mar 12, 2022

Choose a reason for hiding this comment

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

subprocess.CalledProcessError it seems. So:

except subprocess.CalledProcessError as e:
    logging.warning(f'Running lsb_release failed with: {e.output}')

Or something like that?


disks = []

return disks
Copy link
Member

Choose a reason for hiding this comment

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

I think here is something wrong since you return an empty array? Isn't it intended to return output and make it an empty array only on except?

@MichaIng
Copy link
Member

MichaIng commented Mar 12, 2022

@ascl00
Many thanks for your PR. I left some comments/suggestions. Also, please rebase onto current dev branch. It requires quite some changes since diskctl.py move to controls/diskctl.py, update.py to handlers/update.py and init/service files to motioneye/extra/.

@MichaIng MichaIng linked an issue Mar 19, 2022 that may be closed by this pull request
signature=$(echo -n "POST:$uri:$data:$password" | sha1sum | cut -d ' ' -f 1)
# allow for sha1 usage as well as sha1sum
sha1=$(which sha1sum sha1)
signature=$(echo -n "POST:$uri:$data:$password" | $sha1 | cut -d ' ' -f 1)
Copy link
Member

Choose a reason for hiding this comment

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

To assure compatibility with any kind of shell:

Suggested change
signature=$(echo -n "POST:$uri:$data:$password" | $sha1 | cut -d ' ' -f 1)
signature=$(printf '%s' "POST:$uri:$data:$password" | "$sha1" | cut -d ' ' -f 1)

@MichaIng MichaIng linked an issue Mar 22, 2022 that may be closed by this pull request
@MichaIng
Copy link
Member

Looks like there is another command call invalid on FreeBSD: #1060

@Baenwort
Copy link

Baenwort commented Sep 1, 2022

Any hope of this being merged in or should I hope for the Python3 move to capture and correct these FreeBSD issues?

@MichaIng
Copy link
Member

MichaIng commented Sep 1, 2022

This PR has major conflicts. It would need to be recreated based on the new dev branch at best, reducing the changes to the minimal really required ones.

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

Successfully merging this pull request may close these issues.

Running on FreeBSD 12.0 cannot backup config on FreeBSD
5 participants