Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Makefile requires environment variables #19

Open
MPapendrecht opened this issue Jun 17, 2020 · 4 comments
Open

Makefile requires environment variables #19

MPapendrecht opened this issue Jun 17, 2020 · 4 comments

Comments

@MPapendrecht
Copy link

Currently the Makefile requires environment variables to be set for the username and password. I'd like to propose to make these variables optional. If they aren't set we fall back to the pip authentication instead of raising an error. This is also more inline with the netsquid Makefile.
To make sure this is obvious to the user, the INSTALLATION.md could be updated to explain how the user can set these environment variables, or similarly to the netsquid Makefile add an echo statement.

@LMJ-19
Copy link

LMJ-19 commented Jun 19, 2020

Removing the check on NETSQUIDPYPI_USER and NETSQUIDPYPI_PWD still works fine: the user will just be asked to enter the user name and pwd when doing make install (or make requirements.

Note: I added @$(PYTHON3) -m pip install "pip>=19.0" to make requirements since the default pip version in venv is 9 or 6. However, when doing make install this requirement is not checked/installed; so currently it only works when you do: make requirements and then make install or (as mentioned in the NetSquid installmake sure the prerequisite overpip >=19` is met.

A maybe ugly fix would be to add @$(PYTHON3) -m pip install "pip>=19.0" also to the make install command.

@AckslD: what do you think?

PYTHON3      = python3
SOURCEDIR    = netsquid_pingpong
TESTDIR      = tests
EXAMPLES     = examples
RUNEXAMPLES  = ${EXAMPLES}/run_examples.py
PIP_FLAGS    = --extra-index-url=https://${NETSQUIDPYPI_USER}:${NETSQUIDPYPI_PWD}@pypi.netsquid.org
MINCOV       = 0

help:
	@echo "install           Installs the package (editable)."
	@echo "verify            Verifies the installation, runs the linter and tests."
	@echo "tests             Runs the tests."
	@echo "examples          Runs the examples and makes sure they work."
	@echo "open-cov-report   Creates and opens the coverage report."
	@echo "lint              Runs the linter."
	@echo "deploy-bdist      Builds and uploads the package to the netsquid pypi server."
	@echo "bdist             Builds the package."
	@echo "test-deps         Installs the requirements needed for running tests and linter."
	@echo "python-deps       Installs the requirements needed for using the package."
	@echo "docs              Creates the html documentation"
	@echo "clean             Removes all .pyc files."

test-deps:
	@$(PYTHON3) -m pip install -r test_requirements.txt

requirements python-deps:
	@$(PYTHON3) -m pip install "pip>=19.0"
	@$(PYTHON3) -m pip install -r requirements.txt ${PIP_FLAGS}

clean:
	@/usr/bin/find . -name '*.pyc' -delete

lint:
	@$(PYTHON3) -m flake8 ${SOURCEDIR} ${TESTDIR} ${EXAMPLEDIR}

tests:
	@$(PYTHON3) -m pytest --cov=${SOURCEDIR} --cov-fail-under=${MINCOV} tests

open-cov-report:
	@$(PYTHON3) -m pytest --cov=${SOURCEDIR} --cov-report html tests && open htmlcov/index.html

examples:
	@${PYTHON3} ${RUNEXAMPLES} > /dev/null && echo "Examples OK!" || echo "Examples failed!"

docs html:
	@${MAKE} -C docs html

bdist:
	@$(PYTHON3) setup.py bdist_wheel

install: test-deps
	@$(PYTHON3) -m pip install -e . ${PIP_FLAGS}

_clean_dist:
	@/bin/rm -rf dist

deploy-bdist: _clean_dist bdist
	@$(PYTHON3) setup.py deploy

verify: clean test-deps python-deps lint tests examples _verified

_verified:
	@echo "The snippet is verified :)"

.PHONY: clean lint test-deps python-deps tests verify bdist deploy-bdist _clean_dist install open-cov-report examples _check_variables docs

@LMJ-19
Copy link

LMJ-19 commented Jun 19, 2020

This works:

requirements python-deps: _check_pip
	@$(PYTHON3) -m pip install -r requirements.txt ${PIP_FLAGS}

install: _check_pip test-deps 
	@$(PYTHON3) -m pip install -e . ${PIP_FLAGS}

_check_pip:
	@echo "*** Checking the correct version of pip is installed."
	@$(PYTHON3) -m pip install "pip>=19.0"

@AckslD
Copy link
Member

AckslD commented Jun 22, 2020

@LMJ-19 alternatively we can also add requirements to install, similar to how test-deps is added.

@LMJ-19
Copy link

LMJ-19 commented Jun 22, 2020

@LMJ-19 alternatively we can also add requirements to install, similar to how test-deps is added.

yes, I like that!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants