[pacman-dev,1/3] doc: describe when and why the prepare function should be used

Message ID 20190312170114.9291-1-eschwartz@archlinux.org
State Under Review
Headers show
Series [pacman-dev,1/3] doc: describe when and why the prepare function should be used | expand

Commit Message

Eli Schwartz March 12, 2019, 5:01 p.m. UTC
We don't want people to run architecture/OS/makepkg.conf specific
processes during prepare() and in fact it's been observed that makepkg
by design doesn't even run prepare_buildenv() for it, so the prohibition
against this is now baked into makepkg.

Reflect this differentiation in the documentation on just what, exactly,
a prepare() function is.

Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
---

This patch coincides with quequotion's clarification of the options=()
array, so thanks for inspiring me to write this!

I believe that both patches have merit independent of each other.

 doc/PKGBUILD.5.asciidoc | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Allan McRae March 19, 2019, 4:30 a.m. UTC | #1
On 13/3/19 3:01 am, Eli Schwartz wrote:
> We don't want people to run architecture/OS/makepkg.conf specific
> processes during prepare() and in fact it's been observed that makepkg
> by design doesn't even run prepare_buildenv() for it, so the prohibition
> against this is now baked into makepkg.
> 
> Reflect this differentiation in the documentation on just what, exactly,
> a prepare() function is.
> 
> Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
> ---
> 
> This patch coincides with quequotion's clarification of the options=()
> array, so thanks for inspiring me to write this!
> 
> I believe that both patches have merit independent of each other.
> 
>  doc/PKGBUILD.5.asciidoc | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/doc/PKGBUILD.5.asciidoc b/doc/PKGBUILD.5.asciidoc
> index e8ce691f..285627ab 100644
> --- a/doc/PKGBUILD.5.asciidoc
> +++ b/doc/PKGBUILD.5.asciidoc
> @@ -342,9 +342,12 @@ files into the packaging directory, with optional `prepare()`, `build()`, and
>  *prepare() Function*::
>  	An optional `prepare()` function can be specified in which operations to
>  	prepare the sources for building, such as patching, are performed. This
> -	function is run after the source extraction and before the `build()`
> -	function. The `prepare()` function is skipped when source extraction
> -	is skipped.
> +	function is run exactly once, after the source extraction and before the

Why add "exactly once" here?  What is this clarifying?

> +	`build()` function. The `prepare()` function is skipped when source
> +	extraction is skipped. No system-specific or build-specific commands should

what is a system-specific or build-specific command here?

> +	be run during `prepare()` under any circumstances, as they are meant to run
> +	exclusively during `build()`, and features like `buildflags` or `makeflags`
> +	are expressly not available.
>  
>  *build() Function*::
>  	The optional `build()` function is use to compile and/or adjust the source
> 

How about this, which documents by example?

An optional `prepare()` function can be specified in which operations to
prepare the sources for building, such as patching, are performed. For
example, generating configuration files using autoreconf should occur in
prepare(), but running the configure script should happen in build().
This function is run after the source extraction and before the
`build()` function. The `prepare()` function is skipped when source
extraction is skipped.
Eli Schwartz March 19, 2019, 4:43 a.m. UTC | #2
On 3/19/19 12:30 AM, Allan McRae wrote:
> On 13/3/19 3:01 am, Eli Schwartz wrote:
>> We don't want people to run architecture/OS/makepkg.conf specific
>> processes during prepare() and in fact it's been observed that makepkg
>> by design doesn't even run prepare_buildenv() for it, so the prohibition
>> against this is now baked into makepkg.
>>
>> Reflect this differentiation in the documentation on just what, exactly,
>> a prepare() function is.
>>
>> Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
>> ---
>>
>> This patch coincides with quequotion's clarification of the options=()
>> array, so thanks for inspiring me to write this!
>>
>> I believe that both patches have merit independent of each other.
>>
>>  doc/PKGBUILD.5.asciidoc | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/doc/PKGBUILD.5.asciidoc b/doc/PKGBUILD.5.asciidoc
>> index e8ce691f..285627ab 100644
>> --- a/doc/PKGBUILD.5.asciidoc
>> +++ b/doc/PKGBUILD.5.asciidoc
>> @@ -342,9 +342,12 @@ files into the packaging directory, with optional `prepare()`, `build()`, and
>>  *prepare() Function*::
>>  	An optional `prepare()` function can be specified in which operations to
>>  	prepare the sources for building, such as patching, are performed. This
>> -	function is run after the source extraction and before the `build()`
>> -	function. The `prepare()` function is skipped when source extraction
>> -	is skipped.
>> +	function is run exactly once, after the source extraction and before the
> 
> Why add "exactly once" here?  What is this clarifying?

In contrast to the build function, which should be optimally designed to
work when makepkg --noextract carries on a build. But in retrospect,
saying "exactly once" does not clarify this and it should be inferred
since build() is logically an exception.

>> +	`build()` function. The `prepare()` function is skipped when source
>> +	extraction is skipped. No system-specific or build-specific commands should
> 
> what is a system-specific or build-specific command here?

system-specific or build-specific is anything that depends on either the
operating system or the current makepkg environment, so that would
preclude running any sort of configure script, meson, cmake, etc.

>> +	be run during `prepare()` under any circumstances, as they are meant to run
>> +	exclusively during `build()`, and features like `buildflags` or `makeflags`
>> +	are expressly not available.
>>  
>>  *build() Function*::
>>  	The optional `build()` function is use to compile and/or adjust the source
>>
> 
> How about this, which documents by example?
> 
> An optional `prepare()` function can be specified in which operations to
> prepare the sources for building, such as patching, are performed. For
> example, generating configuration files using autoreconf should occur in
> prepare(), but running the configure script should happen in build().
> This function is run after the source extraction and before the
> `build()` function. The `prepare()` function is skipped when source
> extraction is skipped.

I like the mentioning by example, but I would ideally like to mention
the unavailability of buildflags somehow, as this illustrates why
prepare() "must" not be used for configure, rather than "should" not.

Patch

diff --git a/doc/PKGBUILD.5.asciidoc b/doc/PKGBUILD.5.asciidoc
index e8ce691f..285627ab 100644
--- a/doc/PKGBUILD.5.asciidoc
+++ b/doc/PKGBUILD.5.asciidoc
@@ -342,9 +342,12 @@  files into the packaging directory, with optional `prepare()`, `build()`, and
 *prepare() Function*::
 	An optional `prepare()` function can be specified in which operations to
 	prepare the sources for building, such as patching, are performed. This
-	function is run after the source extraction and before the `build()`
-	function. The `prepare()` function is skipped when source extraction
-	is skipped.
+	function is run exactly once, after the source extraction and before the
+	`build()` function. The `prepare()` function is skipped when source
+	extraction is skipped. No system-specific or build-specific commands should
+	be run during `prepare()` under any circumstances, as they are meant to run
+	exclusively during `build()`, and features like `buildflags` or `makeflags`
+	are expressly not available.
 
 *build() Function*::
 	The optional `build()` function is use to compile and/or adjust the source