diff --git a/cms/envs/common.py b/cms/envs/common.py index 3e91dfbaf3..274d230394 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -456,8 +456,11 @@ COURSES_ROOT = ENV_ROOT / "data" GITHUB_REPO_ROOT = ENV_ROOT / "data" +# TODO: Is this next line necessary? sys.path.append(REPO_ROOT) -sys.path.append(PROJECT_ROOT / 'djangoapps') +# TODO: The next two path modifications will be removed in an upcoming Open edX release. +# See docs/decisions/0007-sys-path-modification-removal.rst +sys.path.append(REPO_ROOT / 'sys_path_hacks' / 'studio') sys.path.append(COMMON_ROOT / 'djangoapps') # For geolocation ip database diff --git a/docs/decisions/0007-sys-path-modification-removal.rst b/docs/decisions/0007-sys-path-modification-removal.rst index 25886ad32f..989316980f 100644 --- a/docs/decisions/0007-sys-path-modification-removal.rst +++ b/docs/decisions/0007-sys-path-modification-removal.rst @@ -26,7 +26,7 @@ This deprecation will take place in the following steps: 1. Add a new folder (``sys_path_hacks``) to isolate the code used for deprecation warnings. -2. For every module importable using the ``sys.path`` style (for instance, ``courseware``), duplicate that module structure into the ``sys_path_hacks/lms`` (or ``sys_path_hacks/studio``) directory. Each file in that directory should do a wild-card import of the corresponding ``lms.djangoapps.`` module, and should log a warning indicating where it was imported from. For example, in ``sys_path_hacks/lms/courseware/views/views.py``, it will wild-card import ``from lms.djangoapps.courseware.views.views import *``. The ``sys_path_hacks/clean.sh`` script will generate these files. +2. For every module importable using the ``sys.path`` style (for instance, ``courseware``), duplicate that module structure into the ``sys_path_hacks/lms`` (or ``sys_path_hacks/studio``) directory. Each file in that directory should do a wild-card import of the corresponding ``lms.djangoapps.`` module, and should log a warning indicating where it was imported from. For example, in ``sys_path_hacks/lms/courseware/views/views.py``, it will wild-card import ``from lms.djangoapps.courseware.views.views import *``. The ``sys_path_hacks/un_sys_path.sh`` script will generate these files. 3. The ``sys.path`` modification will be changed to point to ``sys_path_hacks/lms``, rather than ``lms/djangoapps``. At this point, any code that references the modules directly will trigger warnings with logging about where the imports were coming from (to drive future cleanup efforts). The warnings will be instances of ``SysPathHackWarning`` (subclass of ``DeprecationWarning``). diff --git a/lms/envs/common.py b/lms/envs/common.py index 96491ce500..19b4a9548b 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -714,8 +714,10 @@ NODE_MODULES_ROOT = REPO_ROOT / "node_modules" DATA_DIR = COURSES_ROOT -# TODO: Remove the rest of the sys.path modification here and in cms/envs/common.py +# TODO: Is this next line necessary? sys.path.append(REPO_ROOT) +# TODO: The next two path modifications will be removed in an upcoming Open edX release. +# See docs/decisions/0007-sys-path-modification-removal.rst sys.path.append(REPO_ROOT / 'sys_path_hacks' / 'lms') sys.path.append(COMMON_ROOT / 'djangoapps') diff --git a/pylintrc b/pylintrc index 38d962c445..bc3b8dfb3a 100644 --- a/pylintrc +++ b/pylintrc @@ -71,7 +71,7 @@ ignore = ,.git,.tox,migrations,node_modules,.pycharm_helpers persistent = yes load-plugins = edx_lint.pylint,pylint_django,pylint_celery -init-hook = "import sys; sys.path.extend(['sys_path_hacks/lms', 'cms/djangoapps', 'common/djangoapps'])" +init-hook = "import sys; sys.path.extend(['sys_path_hacks/lms', 'sys_path_hacks/studio', 'common/djangoapps'])" [MESSAGES CONTROL] enable = @@ -464,4 +464,4 @@ int-import-graph = [EXCEPTIONS] overgeneral-exceptions = Exception -# 7bfb536a0940715cbd9013a31f37e8c2fc09f16c +# 0981d49b0645874588dcf4423195239dd9e414b7 diff --git a/pylintrc_tweaks b/pylintrc_tweaks index 892c1cdf7c..eec6be8e12 100644 --- a/pylintrc_tweaks +++ b/pylintrc_tweaks @@ -1,7 +1,7 @@ # pylintrc tweaks for use with edx_lint. [MASTER] ignore+ = ,.git,.tox,migrations,node_modules,.pycharm_helpers -init-hook="import sys; sys.path.extend(['sys_path_hacks/lms', 'cms/djangoapps', 'common/djangoapps'])" +init-hook="import sys; sys.path.extend(['sys_path_hacks/lms', 'sys_path_hacks/studio', 'common/djangoapps'])" [MESSAGES CONTROL] disable+ = diff --git a/sys_path_hacks/un_sys_path.sh b/sys_path_hacks/un_sys_path.sh index 1c4fdb14b2..d5f3cc8b45 100755 --- a/sys_path_hacks/un_sys_path.sh +++ b/sys_path_hacks/un_sys_path.sh @@ -1,20 +1,32 @@ #!/usr/bin/env bash -# Example usage: -# ~/edx-platform> sys_path_hacks/un_sys_path.sh lms -# Writing sys_path_hacks/lms/.../xyz.py -# .... -# ~/edx-platform> +# +# Usage: +# +# sys_path_hacks/un_sys_path.sh SOURCE DESTINATION +# +# where SOURCE is the folder from which modules will be recursively copied +# and DESTINATION is the subfolder of `sys_path_hacks` in which they end up. +# +# For example: +# +# ~/edx-platform> sys_path_hacks/un_sys_path.sh common/djangoapps studio +# +# will mirror the packages structure of `common/djangoapps` within `sys_path_hacks/studio`. +# One would run this if they want to mimic the effect of adding 'common/djangoapps' +# to `sys.path` within Studio. -# Shellchecks recommends using search/replace instead of sed. It's fine as is. +# Shellcheck recommends using search/replace instead of sed. It's fine as is. # shellcheck disable=SC2001 set -e set -o pipefail set -u -TARGET="$1" -for path in $(find "${TARGET}/djangoapps/" -name '*.py' | grep -v migrations); do - if [[ "$path" == "${TARGET}/djangoapps/__init__.py" ]]; then +SOURCE="$1" +PYTHON_SOURCE="${SOURCE/\//.}" +DESTINATION="$2" +for path in $(find "${SOURCE}/" -name '*.py' | grep -v migrations); do + if [[ "$path" == "${SOURCE}/__init__.py" ]]; then # Skip unnecessary root __init__.py. continue fi @@ -23,15 +35,19 @@ for path in $(find "${TARGET}/djangoapps/" -name '*.py' | grep -v migrations); d # We've gone to prod with this excluded, and it hasn't been a problem. continue fi - new_path=$(echo "$path" | sed "s#${TARGET}/djangoapps/#sys_path_hacks/${TARGET}/#") + if [[ "$path" == "cms/djangoapps/contentstore/management/commands/import.py" ]]; then + # Also skip this file because its name is problematic for the sys path hack. + continue + fi + new_path=$(echo "$path" | sed "s#${SOURCE}/#sys_path_hacks/${DESTINATION}/#") python_path=$(echo "$path" | sed "s#/#.#g" | sed "s#.py##" | sed "s#.__init__##") - old_python_path=$(echo "$python_path" | sed "s#${TARGET}.djangoapps.##") + old_python_path=$(echo "$python_path" | sed "s#${PYTHON_SOURCE}.##") echo "Writing ${new_path}" mkdir -p "$(dirname "$new_path")" { echo "from sys_path_hacks.warn import warn_deprecated_import" echo - echo "warn_deprecated_import('${TARGET}.djangoapps', '${old_python_path}')" + echo "warn_deprecated_import('${PYTHON_SOURCE}', '${old_python_path}')" echo echo "from ${python_path} import *" } > "$new_path"