add extern keyword on non macOS static variables

[ghost]

Closes https://github.com/wbond/oscrypto/issues/34.

Added extern keyword to macOS SDK non-static variables that pop up in warnings.

[wbond]

Awesome, thanks for taking the time to submit a fix!

I just triggered a rerun of the PyPy tests on Mac on CircleCI.

The other thing we should probably do is add a test that fails to make sure this works. I'm not necessarily expecting you to handle this since it is junk related to CI and the whole setup of the environments there.

From my understanding, to cause the issues, we would need to test with the latest cffi on Mac.

[codecov[bot]]

Codecov Report

Merging #38 into master will decrease coverage by 0.24%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master      #38      +/-   ##
==========================================
- Coverage   84.04%   83.79%   -0.25%     
==========================================
  Files          65       65              
  Lines        9325     9325              
==========================================
- Hits         7837     7814      -23     
- Misses       1488     1511      +23

Impacted Files	Coverage Δ
oscrypto/__init__.py	60.46% <0%> (-10.47%)	:arrow_down:
oscrypto/trust_list.py	79.8% <0%> (-4.81%)	:arrow_down:
oscrypto/_openssl/_libcrypto_ctypes.py	96.25% <0%> (-1.5%)	:arrow_down:
oscrypto/_openssl/tls.py	78.49% <0%> (-1.08%)	:arrow_down:
oscrypto/_mac/tls.py	82.9% <0%> (-0.15%)	:arrow_down:
oscrypto/_win/tls.py	88.52% <0%> (+0.26%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 25deb4a...1877f6f. Read the comment docs.

[ghost]

Awesome, thanks for taking the time to submit a fix!

I just triggered a rerun of the PyPy tests on Mac on CircleCI.

The other thing we should probably do is add a test that fails to make sure this works. I'm not necessarily expecting you to handle this since it is junk related to CI and the whole setup of the environments there.

From my understanding, to cause the issues, we would need to test with the latest cffi on Mac.

No problem! I would like to finish what I started, and it'd be a great opportunity for me to learn about CI stuff including different environments, so I could add some tests.

I'll take a look at the tests already implemented and we can discuss how to implement the tests, but my initial thoughts are, should we test for no warnings with the latest cffi?

I'm thinking, there might be a cleaner way with pre-existing tools for this, such as separating the header file content to dedicated files and run a grammar check on them with a compiler. For introducing conditional code blocks, as it is in _security_cffi.py,, we could also move those blocks into their own files we could utilize file naming to notate version interval starts.

Let me know what you think!

[wbond]

No problem! I would like to finish what I started, and it'd be a great opportunity for me to learn about CI stuff including different environments, so I could add some tests.

I'll take a look at the tests already implemented and we can discuss how to implement the tests, but my initial thoughts are, should we test for no warnings with the latest cffi?

There will be two phases to getting this change tested. The first will be to figure out how to detect the warnings that cffi spits out about extern. You'll probably need to use the warnings module to capture them, at least I am guessing there is probably a way to do that. You can do this all on a machine that reproduced the problem.

The second and more complicated part is going to get getting the custom oscrypto task and CI runner to:

1. Change the CI tests to always tell oscrypto to use ctypes
2. Create an environmental variable to tell oscrypto to use cffi. There is an example of this to tell oscrypto to use OpenSSL on Mac. Look in run.py for the entry point. After that you'll find most of the infrastructure for developing the testing the package is in the dev/ folder.
3. Update requires/ci to require cffi and its dependencies. We'll likely need to guard these with a conditional to only install them on specific versions of Python. Some of the other requires files have such version guards. This is because lots of the Python versions we test won't have wheels for cffi, and the CI tasks doesn't use pip for installing packages. pip has dropped support for many of the versions and platforms we test on and support, so there is a home-grown minimal implementation of package downloading/installing in dev/deps.py. You can see the versions with wheels at https://pypi.org/project/cffi/#files. It looks like targeting Python 3.7+ should be fine since there are wheels for 3.7 and 3.8 for all platforms we support.
4. Update the circleci (and maybe GitHub Actions) CI configs to add a new run that will test with cffi.

I've glossed over some of the details of the CI changes, but if you are unfamiliar with CI that should at least get you started on the process.

[wbond]

I had a little free time and got the CI config to a point where this fix would cause tests to pass.

Thanks for your work in getting these changes in place!

[ghost]

Awesome, thanks! I was busy so couldn't get around to this. I'll take a look at your solution.