#32734 closed Bug (fixed)
django-admin startapp with trailing slash in directory name results in error
Reported by: | jooadam | Owned by: | Rohith P R |
---|---|---|---|
Component: | Utilities | Version: | 3.2 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
Bash tab-completion appends trailing slashes to directory names. django-admin startapp name directory/
results in the error:
CommandError: '' is not a valid app directory. Please make sure the directory is a valid identifier.
The error is caused by line 77 of django/core/management/templates.py
by calling basename()
on the path with no consideration for a trailing slash:
self.validate_name(os.path.basename(target), 'directory')
Removing potential trailing slashes would solve the problem:
self.validate_name(os.path.basename(target.rstrip(os.sep)), 'directory')
Change History (13)
comment:1 by , 4 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 4 years ago
I didn't look into exactly why but it works for startproject
This is the relevant piece of code:
if app_or_project == 'app': self.validate_name(os.path.basename(target), 'directory')
The changes were made here: https://github.com/django/django/pull/11270/files
follow-up: 4 comment:3 by , 4 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:4 by , 4 years ago
Owner: | changed from | to
---|
Replying to Rohith P R:
Rohith, sorry for not assigning it to me first, I just submitted a pull request: https://github.com/django/django/pull/14383
follow-up: 6 comment:5 by , 4 years ago
Hey @jooadam! I already had a [PR](https://github.com/django/django/pull/14382) open to fix this issue. I left a comment on your PR btw. :)
comment:6 by , 4 years ago
Replying to Rohith P R:
Hey @jooadam! I already had a [PR](https://github.com/django/django/pull/14382) open to fix this issue. I left a comment on your PR btw. :)
Yeah, I saw it after the fact :/
comment:7 by , 4 years ago
Owner: | changed from | to
---|
comment:8 by , 4 years ago
Patch needs improvement: | set |
---|
Regression in fc9566d42daf28cdaa25a5db1b5ade253ceb064f.
comment:9 by , 4 years ago
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
follow-up: 11 comment:10 by , 4 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
In 530f58ca:
comment:11 by , 4 years ago
Replying to Mariusz Felisiak <felisiak.mariusz@…>:
Mariusz, just wanted to say that I spent about four hours on my first contribution to the project, that you just completely threw away here. I probably won’t have a second.
comment:12 by , 4 years ago
jooadam, it's a shame that your first contributing experience was not great and to hear that you're considering not repeating the experience but please be fair to Mariusz.
There was a miscommunication between multiple parties interested in helping due to the lack of immediate assignment of an owner and Mariusz has little to do with that. I'm pretty sure Rohith also invested quite a lot of time in getting their solution ready to address an an easy picking issue and just happened to follow the contributing guidelines. In the end only a single patch can land.
I sincerely hope you reconsider your decision and realize these hours were not completely wasted; you've certainly learned a few things about Django and the contributing process along the way.
comment:13 by , 3 years ago
Does any plan backport https://github.com/django/django/pull/11270 and https://github.com/django/django/pull/14382 to branch stable/2.2.x and stable/3.2.x.
# https://github.com/django/django/pull/11270 (base) root@69faeb96:/tmp/123321/django# git branch -a --contains fc9566d42daf28cdaa25a5db1b5ade253ceb064f * main stable/3.2.x remotes/origin/HEAD -> origin/main remotes/origin/main remotes/origin/make-zoneinfo-default-timezone-implementation remotes/origin/stable/3.0.x remotes/origin/stable/3.1.x remotes/origin/stable/3.2.x # https://github.com/django/django/pull/14382 (base) root@69faeb96-0f43-442b-ad2d-3e5cfa31f6d5:/tmp/123321/django# git branch -a --contains 530f58caaa5052e9e56bf8461caee4d821953bcb * main remotes/origin/HEAD -> origin/main remotes/origin/main remotes/origin/make-zoneinfo-default-timezone-implementation
Or, startapp will crash bug since PR https://github.com/django/django/pull/11270:
(base) root@69faeb96:/tmp# /tmp/venv/bin/python -m django --version 3.2.6 (base) root@69faeb96:/tmp# /tmp/venv/bin/python -m django startproject my_project -v3 && cd my_project Rendering project template files with extensions: .py Rendering project template files with filenames: Creating /tmp/my_project/manage.py Creating /tmp/my_project/my_project/__init__.py Creating /tmp/my_project/my_project/asgi.py Creating /tmp/my_project/my_project/settings.py Creating /tmp/my_project/my_project/urls.py Creating /tmp/my_project/my_project/wsgi.py (base) root@69faeb96:/tmp/my_project# mkdir -p my_app && cd my_app (base) root@69faeb96:/tmp/my_project/my_app# /tmp/venv/bin/python -m django startapp my_app . -v3 CommandError: '.' is not a valid app directory. Please make sure the directory is a valid identifier.
However, that's ok if startapp with other same method:
(base) root@69faeb96:/tmp/my_project/my_app# cd .. (base) root@69faeb96:/tmp/my_project# ls manage.py my_app my_project (base) root@69faeb96:/tmp/my_project# rm -rf my_app/ (base) root@69faeb96:/tmp/my_project# ls manage.py my_project (base) root@69faeb96:/tmp/my_project# /tmp/venv/bin/python -m django startapp my_app -v3 Rendering app template files with extensions: .py Rendering app template files with filenames: Creating /tmp/my_project/my_app/__init__.py Creating /tmp/my_project/my_app/admin.py Creating /tmp/my_project/my_app/apps.py Creating /tmp/my_project/my_app/models.py Creating /tmp/my_project/my_app/tests.py Creating /tmp/my_project/my_app/views.py Creating /tmp/my_project/my_app/migrations/__init__.py
OK, yes, this seems a case we could handle.
I didn't look into exactly why but it works for
startproject
:Thanks for the report. Do you fancy making a PR?