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

Update pycryptodome version #11866

Merged
merged 1 commit into from
Dec 16, 2019
Merged

Conversation

urutva
Copy link
Contributor

@urutva urutva commented Nov 14, 2019

Description

Summary of change

The required version of pycryptodome currently is either 3.7.2 or 3.7.3.
But there is no wheel package for these version for latest version of python
(3.8). Due to this, pip will download source and compile while trying to
install pycryptodome. On Windows, there is no c, c++ compiler installed
by default which leads to pip asking users to download and install
Microsoft Visual C++ Build Tools.

The release version 3.9.3 contains wheel package for python 3.8 on
all supported operating systems.

Signed-off-by: Devaraj Ranganna devaraj.ranganna@arm.com

Documentation

None


Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers (optional)


Release Notes (required for feature/major PRs)

Summary of changes
Impact of changes
Migration actions required

Fixes #11787

@ciarmcom ciarmcom requested a review from a team November 14, 2019 14:00
@ciarmcom
Copy link
Member

@Devran01, thank you for your changes.
@ARMmbed/mbed-os-maintainers please review.

Copy link
Member

@bulislaw bulislaw left a comment

Choose a reason for hiding this comment

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

No issues on Mbed side, how about @ARMmbed/mbed-os-tools ?

@0xc0170 0xc0170 requested a review from a team November 15, 2019 17:48
@0xc0170
Copy link
Contributor

0xc0170 commented Nov 15, 2019

@ARMmbed/mbed-os-test This might require an update in our CI as well, please review

I'll run initial CI that might fail

@mbed-ci
Copy link

mbed-ci commented Nov 15, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

Copy link
Contributor

@mark-edgeworth mark-edgeworth left a comment

Choose a reason for hiding this comment

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

Need to see evidence of testing on windows, mac and Linux; also on the version of GCC, ARM CC and IAR compilers that we are currently using.

requirements.txt Outdated
@@ -19,7 +19,7 @@ beautifulsoup4>=4,<=4.6.3
pyelftools>=0.24,<=0.25
manifest-tool==1.5.2
icetea>=1.2.1,<1.3
pycryptodome>=3.7.2,<=3.7.3
pycryptodome==3.9.3
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest using >=3.9.3,<4 here, rather than pinning the version

Suggested change
pycryptodome==3.9.3
pycryptodome>=3.9.3,<4

Copy link
Contributor Author

@urutva urutva Nov 19, 2019

Choose a reason for hiding this comment

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

@mark-edgeworth Agreed. Fixed.

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 19, 2019

Need to see evidence of testing on windows, mac and Linux; also on the version of GCC, ARM CC and IAR compilers that we are currently using.

We should be able to run at least this one in CI (manual run). @VeliMattiLahtela should be able to help to get this tested

@Devran01 Please talk to him

The required version of pycryptodome currently is either 3.7.2 or 3.7.3.
But there is no wheel package for these version for latest version of python
(3.8). Due to this, pip will download source and compile while trying to
install pycryptodome. On Windows, there is no c, c++ compiler installed
by default which leads to pip asking users to download and install
Microsoft Visual C++ Build Tools.

The release version 3.9.3 contains wheel package for python 3.8 on
all supported operating systems.

Signed-off-by: Devaraj Ranganna <devaraj.ranganna@arm.com>
@urutva urutva force-pushed the update_pycryptodome_version branch from d2833ba to 98ef548 Compare November 19, 2019 16:07
Copy link
Contributor

@mark-edgeworth mark-edgeworth left a comment

Choose a reason for hiding this comment

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

I'm approving this because it appears to offer better python 3 support and the docs suggest that the platform specific code is now included (it wasn't in the previous release). I've not tested this.

@urutva
Copy link
Contributor Author

urutva commented Nov 20, 2019

@0xc0170 Travis failure is caused by temporary failure ConnectionResetError: [Errno 104] Connection reset by peer while downloading pycryptodome-3.9.4. Can you please restart the Travis job?

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 20, 2019

Travis restarted

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 22, 2019

As discussed, this is not ready for 5.15, moving to 6.0

@0xc0170 0xc0170 added release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 and removed release-version: 5.15.0-rc1 labels Nov 22, 2019
@adbridge
Copy link
Contributor

CI started

@mbed-ci
Copy link

mbed-ci commented Dec 13, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 2
Build artifacts

@0xc0170 0xc0170 merged commit 4aeaab4 into ARMmbed:master Dec 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dependency on Microsoft Visual C++ Build Tools
7 participants