[Launchpad-reviewers] [Merge] ~ines-almeida/txpkgupload:refactor-makefile-commands into txpkgupload:master

2023-07-07 Thread Ines Almeida
Ines Almeida has proposed merging 
~ines-almeida/txpkgupload:refactor-makefile-commands into txpkgupload:master.

Commit message:
Refactor Makefile logic

This will ensure that the dependencies repository isn't cloned on every command 
that builds the virtual env which should be unnecessary in a few cases, e.g., 
when the txpkgupload charm builds from the tarball (given the tarball should 
already contain the dependencies)

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/txpkgupload/+git/txpkgupload/+merge/446311
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/txpkgupload:refactor-makefile-commands into txpkgupload:master.
diff --git a/Makefile b/Makefile
index 55d04d0..bc69913 100644
--- a/Makefile
+++ b/Makefile
@@ -36,10 +36,14 @@ SWIFT_OBJECT_PATH = \
 	$(SWIFT_CONTAINER_NAME)/$(TARBALL_BUILD_LABEL)/$(TARBALL_FILE_NAME)
 
 
-default: compile
+default: bootstrap
 
 
-bootstrap compile:
+bootstrap: $(DEPENDENCY_DIR)
+	$(MAKE) compile
+
+
+compile:
 	$(MAKE) $(ENV)
 	[ ! -d .git ] || $(MAKE) $(VERSION_INFO)
 
@@ -68,7 +72,7 @@ $(DEPENDENCY_DIR):
 	git clone $(DEPENDENCY_REPO) $(DEPENDENCY_DIR)
 
 
-$(ENV): | $(DEPENDENCY_DIR)
+$(ENV):
 	mkdir -p $(ENV)
 	(echo '[easy_install]'; \
 	 echo 'find_links = file://$(DEPENDENCY_DIR)/') \
@@ -105,8 +109,7 @@ clean: clean_pip
 build-tarball:
 	@echo "Creating deployment tarball at $(TARBALL_BUILD_PATH)"
 	rm -rf $(ENV)
-	$(MAKE) $(ENV)
-	$(MAKE) $(VERSION_INFO)
+	$(MAKE) boorstrap
 	$(PIP) wheel -f $(DEPENDENCY_DIR) --no-index -w $(WHEELS) \
 		-r bootstrap-requirements.txt -r requirements.txt
 	mkdir -p $(TARBALL_BUILD_DIR)
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp


[Launchpad-reviewers] [Merge] ~ines-almeida/txpkgupload:refactor-makefile-commands into txpkgupload:master

2023-07-10 Thread Ines Almeida
Ines Almeida has proposed merging 
~ines-almeida/txpkgupload:refactor-makefile-commands into txpkgupload:master.

Commit message:
Install python packages from wheels when wheels directory exist

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/txpkgupload/+git/txpkgupload/+merge/446377

When building the 'env', it installs from the `wheels` directory when the 
directory exists - this assumes that if the `wheels` directory exists, it will 
contain the wheels we need, which I think it's a fair assumption.

I tested both the cases where there is a wheels directory with all the wheels, 
and where there isn't a wheels directory.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/txpkgupload:refactor-makefile-commands into txpkgupload:master.
diff --git a/Makefile b/Makefile
index 98163b0..7f1eed2 100644
--- a/Makefile
+++ b/Makefile
@@ -74,13 +74,18 @@ $(DEPENDENCY_DIR):
 
 $(ENV):
 	mkdir -p $(ENV)
+	$(PYTHON) -m venv env
+	@if [ -d $(WHEELS) ]; then \
+		echo 'Found $(WHEELS) director, installing from folder...'; \
+		$(PIP) install -f $(WHEELS) --no-index -r bootstrap-requirements.txt; \
+		$(PIP) install -f $(WHEELS) --no-index -c requirements.txt -e '.[test]'; \
+	else \
 	(echo '[easy_install]'; \
 	 echo 'find_links = file://$(DEPENDENCY_DIR)/') \
-		>$(ENV)/.pydistutils.cfg
-	VIRTUALENV_SETUPTOOLS=1 $(VIRTUALENV) \
-		--python=$(PYTHON) --never-download $(ENV)
-	$(PIP) install $(PIP_ARGS) -r bootstrap-requirements.txt
-	$(PIP) install $(PIP_ARGS) -c requirements.txt -e '.[test]'
+		>$(ENV)/.pydistutils.cfg; \
+		$(PIP) install $(PIP_ARGS) -r bootstrap-requirements.txt; \
+		$(PIP) install $(PIP_ARGS) -c requirements.txt -e '.[test]'; \
+	fi
 	$(ENV)/bin/python -m compileall -q src
 	ln -nsf $(ENV)/bin bin
 
@@ -100,6 +105,7 @@ clean: clean_pip
 	$(RM) $(VERSION_INFO)
 	$(RM) -rf dist
 	$(RM) -rf build
+	$(RM) -rf $(ENV)
 
 
 .PHONY: compile check clean clean_pip default dist
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp


Re: [Launchpad-reviewers] [Merge] ~ines-almeida/txpkgupload:refactor-makefile-commands into txpkgupload:master

2023-07-07 Thread Guruprasad



Diff comments:

> diff --git a/Makefile b/Makefile
> index 55d04d0..bc69913 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -105,8 +109,7 @@ clean: clean_pip
>  build-tarball:
>   @echo "Creating deployment tarball at $(TARBALL_BUILD_PATH)"
>   rm -rf $(ENV)
> - $(MAKE) $(ENV)
> - $(MAKE) $(VERSION_INFO)
> + $(MAKE) boorstrap

`s/boorstrap/bootstrap`

>   $(PIP) wheel -f $(DEPENDENCY_DIR) --no-index -w $(WHEELS) \
>   -r bootstrap-requirements.txt -r requirements.txt
>   mkdir -p $(TARBALL_BUILD_DIR)


-- 
https://code.launchpad.net/~ines-almeida/txpkgupload/+git/txpkgupload/+merge/446311
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/txpkgupload:refactor-makefile-commands into txpkgupload:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp


Re: [Launchpad-reviewers] [Merge] ~ines-almeida/txpkgupload:refactor-makefile-commands into txpkgupload:master

2023-07-10 Thread Colin Watson
This seems unnecessarily complicated.  `PIP_FIND_LINKS` can take multiple 
entries, and it isn't a problem if one of them doesn't exist.

Instead of this change, what happens if you:

 * add `--extra-search-dir=$(CURDIR)/wheels/ 
--extra-search-dir=$(DEPENDENCY_DIR)/` to the `$(VIRTUALENV)` command
 * change `PIP_FIND_LINKS=file://$(DEPENDENCY_DIR)/` to 
`PIP_FIND_LINKS="file://$(CURDIR)/wheels/ file://$(DEPENDENCY_DIR)/"`

?  I think that should work, and it's more in line with how Launchpad itself 
does things.
-- 
https://code.launchpad.net/~ines-almeida/txpkgupload/+git/txpkgupload/+merge/446377
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/txpkgupload:refactor-makefile-commands into txpkgupload:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp


Re: [Launchpad-reviewers] [Merge] ~ines-almeida/txpkgupload:refactor-makefile-commands into txpkgupload:master

2023-07-10 Thread Ines Almeida
Thank you for the suggestion! I updated the code, it seems to work nicely
-- 
https://code.launchpad.net/~ines-almeida/txpkgupload/+git/txpkgupload/+merge/446377
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/txpkgupload:refactor-makefile-commands into txpkgupload:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp