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

CI fixes / package build fixes #19

Merged
merged 2 commits into from
Apr 18, 2022
Merged

CI fixes / package build fixes #19

merged 2 commits into from
Apr 18, 2022

Conversation

a68366
Copy link
Contributor

@a68366 a68366 commented Apr 17, 2022

Fixes python package build.

I've also enabled win32 build, since currently published v0.2 had it. It shouldn't be a problem, unless the rust code is somehow tied to a specific architecture, i guess. Tried using it on win 7 x32, it worked fine.

Build log is available here: https://github.com/a68366/cryptg/actions/runs/2181002754
And the resulting packages are here: https://test.pypi.org/project/cryptgtest/0.3.1/

Closes #15
Closes #17

@Lonami
Copy link
Collaborator

Lonami commented Apr 17, 2022

Thanks, since this removes (and not moves) licensing info I'll wait on @cher-nov to give the thumbs up before merging.

@cher-nov
Copy link
Owner

Thanks, since this removes (and not moves) licensing info I'll wait on @cher-nov to give the thumbs up before merging.

This is definitely bad, and I would strongly disapprove such solution, sorry.

@Lonami
Copy link
Collaborator

Lonami commented Apr 17, 2022

I'm not sure what the implications on removing those files from there actually are. I would have to do some research before I can argue in favour of this too.

This change fixes package build.
Also include third party license files in source distribution.
@a68366
Copy link
Contributor Author

a68366 commented Apr 17, 2022

Force-pushed the branch. I used layout similair to what e.g. pandas (see LICENSES) and ansible (licenses) do.

Since you want to have 3rd party lincenses in the repo, I assume that you also want to ship them with source distribution, so I included the directory in MANIFEST.in.

@Lonami
Copy link
Collaborator

Lonami commented Apr 17, 2022

This seems like the best of both worlds, @cher-nov what do you think of it now?

@cher-nov
Copy link
Owner

This seems like the best of both worlds, @cher-nov what do you think of it now?

Let's accept until I have some time to finally get to this project and put things in order.

@Lonami Lonami merged commit a58ad72 into cher-nov:master Apr 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Master branch not being installed Cannot build from this repo even with all requirements satisfied
3 participants