-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Decouple project from the existence of Pipfile. #3386
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Do not touch Pipfile early and rely on it so that one can do ``pipenv sync`` without a Pipfile. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Improve the error message when one tries to initialize a Pipenv project under ``/``. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -539,11 +539,16 @@ def ensure_project( | |
# Automatically use an activated virtualenv. | ||
if PIPENV_USE_SYSTEM: | ||
system = True | ||
if not project.pipfile_exists: | ||
if deploy is True: | ||
raise exceptions.PipfileNotFound | ||
else: | ||
project.touch_pipfile() | ||
if not project.pipfile_exists and deploy: | ||
raise exceptions.PipfileNotFound | ||
# Fail if working under / | ||
if not project.name: | ||
click.echo( | ||
"{0}: Pipenv is not intended to work under the root directory, " | ||
"please choose another path.".format(crayons.red("ERROR")), | ||
err=True | ||
) | ||
sys.exit(1) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't touch an empty Pipfile here. |
||
# Skip virtualenv creation when --system was used. | ||
if not system: | ||
ensure_virtualenv( | ||
|
@@ -606,24 +611,23 @@ def shorten_path(location, bold=False): | |
def do_where(virtualenv=False, bare=True): | ||
"""Executes the where functionality.""" | ||
if not virtualenv: | ||
location = project.pipfile_location | ||
# Shorten the virtual display of the path to the virtualenv. | ||
if not bare: | ||
location = shorten_path(location) | ||
if not location: | ||
if not project.pipfile_exists: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So this isn’t different fundamentally. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
click.echo( | ||
"No Pipfile present at project home. Consider running " | ||
"{0} first to automatically generate a Pipfile for you." | ||
"".format(crayons.green("`pipenv install`")), | ||
err=True, | ||
) | ||
elif not bare: | ||
return | ||
location = project.pipfile_location | ||
# Shorten the virtual display of the path to the virtualenv. | ||
if not bare: | ||
location = shorten_path(location) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
click.echo( | ||
"Pipfile found at {0}.\n Considering this to be the project home." | ||
"".format(crayons.green(location)), | ||
err=True, | ||
) | ||
pass | ||
else: | ||
click.echo(project.project_directory) | ||
else: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,7 +54,7 @@ def _normalized(p): | |
path_str = matches and matches[0] or str(loc) | ||
else: | ||
path_str = str(loc) | ||
return normalize_drive(path_str) | ||
return normalize_drive(os.path.abspath(path_str)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don’t change this one, it is going to regress environment discovery on windows unless you change every reference to it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a relative path also breaks https://github.com/pypa/pipenv/blob/master/pipenv/project.py#L217-L221 |
||
|
||
|
||
DEFAULT_NEWLINES = u"\n" | ||
|
@@ -226,7 +226,7 @@ def name(self): | |
|
||
@property | ||
def pipfile_exists(self): | ||
return bool(self.pipfile_location) | ||
return os.path.isfile(self.pipfile_location) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check the existence of the file instead There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don’t use Improvements are good and important and I’m willing to break compatibility for some things. But changes like this one only screw users over and make people like Hynek write about how many regressions they experience There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change is made to work with #3386 (diff) |
||
|
||
@property | ||
def required_python_version(self): | ||
|
@@ -241,11 +241,7 @@ def required_python_version(self): | |
|
||
@property | ||
def project_directory(self): | ||
if self.pipfile_location is not None: | ||
return os.path.abspath(os.path.join(self.pipfile_location, os.pardir)) | ||
|
||
else: | ||
return None | ||
return os.path.abspath(os.path.join(self.pipfile_location, os.pardir)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Return the project home even if there is no Pipfile existing. NOTE: |
||
|
||
@property | ||
def requirements_exists(self): | ||
|
@@ -259,8 +255,7 @@ def is_venv_in_project(self): | |
|
||
@property | ||
def virtualenv_exists(self): | ||
# TODO: Decouple project from existence of Pipfile. | ||
if self.pipfile_exists and os.path.exists(self.virtualenv_location): | ||
if os.path.exists(self.virtualenv_location): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So |
||
if os.name == "nt": | ||
extra = ["Scripts", "activate.bat"] | ||
else: | ||
|
@@ -478,7 +473,7 @@ def pipfile_location(self): | |
try: | ||
loc = pipfile.Pipfile.find(max_depth=PIPENV_MAX_DEPTH) | ||
except RuntimeError: | ||
loc = None | ||
loc = "Pipfile" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’m pretty sure this returns an actual path like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In case the Pipfile is not found it returns None. Then https://github.com/pypa/pipenv/blob/master/pipenv/project.py#L217-L221 will be broken. |
||
self._pipfile_location = _normalized(loc) | ||
return self._pipfile_location | ||
|
||
|
@@ -507,6 +502,8 @@ def parsed_pipfile(self): | |
|
||
def read_pipfile(self): | ||
# Open the pipfile, read it into memory. | ||
if not self.pipfile_exists: | ||
return "" | ||
with io.open(self.pipfile_location) as f: | ||
contents = f.read() | ||
self._pipfile_newlines = preferred_newlines(f) | ||
|
@@ -664,11 +661,6 @@ def dev_packages(self): | |
"""Returns a list of dev-packages, for pip-tools to consume.""" | ||
return self._build_package_list("dev-packages") | ||
|
||
def touch_pipfile(self): | ||
"""Simply touches the Pipfile, for later use.""" | ||
with open("Pipfile", "a"): | ||
os.utime("Pipfile", None) | ||
|
||
@property | ||
def pipfile_is_empty(self): | ||
if not self.pipfile_exists: | ||
|
@@ -685,8 +677,7 @@ def create_pipfile(self, python=None): | |
ConfigOptionParser, make_option_group, index_group | ||
) | ||
|
||
name = self.name if self.name is not None else "Pipfile" | ||
config_parser = ConfigOptionParser(name=name) | ||
config_parser = ConfigOptionParser(name=self.name) | ||
config_parser.add_option_group(make_option_group(index_group, config_parser)) | ||
install = config_parser.option_groups[0] | ||
indexes = ( | ||
|
@@ -839,7 +830,7 @@ def write_lockfile(self, content): | |
|
||
@property | ||
def pipfile_sources(self): | ||
if "source" not in self.parsed_pipfile: | ||
if self.pipfile_is_empty or "source" not in self.parsed_pipfile: | ||
return [DEFAULT_SOURCE] | ||
# We need to make copies of the source info so we don't | ||
# accidentally modify the cache. See #2100 where values are | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do still want to create non existent pipfiles when we ensure projects exist, no?