securedrop icon indicating copy to clipboard operation
securedrop copied to clipboard

Remove `sh` dependency

Open legoktm opened this issue 2 years ago • 4 comments

This is a good first issue for new contributors to take on, if you have any questions, please ask on the task or in our Gitter room!

Description

sh is a pretty cool project, it allows you to shell out to programs as if they were native Python programs. This comes with a downside, it obfuscates the underlying command being run, and it's an extra dependency.

Here are all the places we use it:

Targets
    Occurrences of 'from sh import' in Project
Found Occurrences in Project  (4 usages found)
    Unclassified  (4 usages found)
        securedrop  (4 usages found)
            securedrop  (1 usage found)
                i18n_tool.py  (1 usage found)
                    19 from sh import git, msgfmt, msgmerge, pybabel, sed, xgettext
            securedrop/tests  (3 usages found)
                test_i18n.py  (1 usage found)
                    34 from sh import pybabel, sed
                test_i18n_tool.py  (1 usage found)
                    13 from sh import git, msginit, pybabel, sed, touch
                test_template_filters.py  (1 usage found)
                    13 from sh import pybabel

Given how minimal its usage is (just i18n_tool.py plus tests), I propose we replace it with explicit subprocess.check_output(...) calls. The biggest advantage of this removal is that pybabel will be the only dependency needed to compile translations at package build time.

legoktm avatar Sep 15 '22 21:09 legoktm

Hi @legoktm , I would love to work on this issue! Has it been assigned to anyone yet? Thanks!

lb803 avatar Sep 21 '22 19:09 lb803

Nope, go for it! Let me know if you have any questions.

legoktm avatar Sep 21 '22 19:09 legoktm

Before seeing this issue, I had already started removing usages of sh.sed() in code that I was touching. I have opened a PR that removes calls to sed(): https://github.com/freedomofpress/securedrop/pull/6562

I will not work on other usages of sh so this issue is still available.

nabla-c0d3 avatar Sep 24 '22 10:09 nabla-c0d3

OK, thanks for the heads up! I won't touch those calls, then.

lb803 avatar Sep 24 '22 11:09 lb803