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

Fix local build and install #620

Closed

Conversation

souljang
Copy link

  • Build environment: Python 3.6.9 on Ubuntu 18.04

  • Issue: Local build and install of stix-shifter didn't work properly as importing stix-shifter-modules.* failed after following python3 setup.py install.

    • Issue 1: Some stix-shifter-modules generated .egg zipfiles while others generated .egg directories. Missing zip-safe flag resulted in inconsistent module installation (https://setuptools.readthedocs.io/en/latest/deprecated/python_eggs.html#zip-safe-and-not-zip-safe), and failure of importing native namespace packages (https://packaging.python.org/guides/packaging-namespace-packages/#creating-a-namespace-package).
    • Fix 1: Set zip-safe flag to False
    • Issue 2: Installing as separate .egg directories caused incorrect path resolution at importing. For example, after installation, according to the order in the path (e.g., easy-install.pth), all stix-shifter-modules.* imports look for the first one in the path (e.g., ./stix_shifter_modules_alertflex-1.0.0-py3.6.egg) regardless of their actual locations.
    • Fix 2: Mode 3 of installation is needed to have a proper installation structure. Change the default installation mode to 3 and fix the function to fill the connectors properly

@CLAassistant
Copy link

CLAassistant commented May 29, 2021

CLA assistant check
All committers have signed the CLA.

@mdazam1942 mdazam1942 self-requested a review May 31, 2021 14:48
# The mode determines how the stix-shifter is packaged
# 1 = Include everything in 1 whl package
# 3 - 3 whl packages respectively for stix-shifter, stix-shifter-utils and stix-shifter-modules
# N - stix-shifter, stix-shifter-utils, and each connector is packaged separately
# <module name> - package only the specified connector

mode = 'N'
mode = '3'
Copy link
Collaborator

Choose a reason for hiding this comment

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

you should not change the default packaging mode, why are you using mode 3 at all? maybe mode 1 will fit you better?

Copy link
Author

Choose a reason for hiding this comment

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

Agreed. Changing the default mode might be too much depending on the workflow. For the distribution purpose for developers (python setup.py bdist_wheel), it would be good to have separate packages.

But, for local build/install for users (python setup.py install), mode 3 or 1 is needed as the installed packages under site-packages will have the mix of egg zipfiles and directories for each stix_shifter_module, which will cause importing errors. For example, after python setup.py install (with mode N), importing qradar module (import stix_shifter_modules.qradar) from outside of the stix-shifter root directory will cause ModuleNotFoundError due to above issues.

Copy link
Author

Choose a reason for hiding this comment

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

By the way, MODE 1/3 bdist/install fails due to incorrect path resolution for stix_shifter_modules. Proper fill_connectors is needed to support MODE 1/3.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@mdazam1942 is looking into it.

@baulus baulus mentioned this pull request Jan 17, 2022
@baulus
Copy link
Contributor

baulus commented Jan 17, 2022

Created another PR with the changes from jiyongj:local_build into branch opencybersecurityalliance:local-build.
This way it will be easier to make new changes in opencybersecurityalliance repo.
I suggest to switch to the new branch opencybersecurityalliance:local-build and close this PR.
New PR #779

@mdazam1942
Copy link
Member

closing this PR since the the same commits are merged through a updated #779

@mdazam1942 mdazam1942 closed this Feb 3, 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.

5 participants