[PATCH cygport 0/3] cygport patches

classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH cygport 0/3] cygport patches

Jon TURNEY
Jon Turney (3):
  Pass DEPEND through to .hint for source package as build-depends:
  Don't allow SRC_URI or PATCH_URI to depend on ARCH
  Update documentation of all and almostall

 README              |  4 ++--
 bin/cygport.in      | 26 ++++++++++++++++++++++++++
 lib/help.cygpart    |  3 ++-
 lib/pkg_pkg.cygpart |  5 +++++
 4 files changed, 35 insertions(+), 3 deletions(-)

--
2.12.2

Reply | Threaded
Open this post in threaded view
|

[PATCH cygport 1/3] Pass DEPEND through to .hint for source package as build-depends:

Jon TURNEY
Converting a dependency atom to a package name with full generality requires
a database of all the pathnames contained in all packages, so we don't even
try to do that here, leaving that task for downstream processing of the
.hint file...
---
 lib/pkg_pkg.cygpart | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/pkg_pkg.cygpart b/lib/pkg_pkg.cygpart
index 5552040..01dec69 100644
--- a/lib/pkg_pkg.cygpart
+++ b/lib/pkg_pkg.cygpart
@@ -710,6 +710,10 @@ _EOF
  cat >> ${distdir}/${PN}/${distsubdir}/${pkg_name[${n}]}-${PVR}.hint <<-_EOF
 external-source: ${PN}
 _EOF
+ else
+ cat >> ${distdir}/${PN}/${distsubdir}/${pkg_name[${n}]}-${PVR}.hint <<-_EOF
+build-depends: ${DEPEND}
+_EOF
  fi
  if defined ${pkg_message_var}
  then
@@ -810,6 +814,7 @@ _EOF
  cat > ${distdir}/${PN}/${PN}-${PVR}.hint <<-_EOF
 category: ${!pkg_category_var:-${CATEGORY}}
 requires:
+build-depends: ${DEPEND}
 sdesc: "${!pkg_summary_var:-${SUMMARY}}"
 ldesc: "${!pkg_description_var:-${DESCRIPTION:-${!pkg_summary_var:-${SUMMARY}}}}"
 skip:
--
2.12.2

Reply | Threaded
Open this post in threaded view
|

[PATCH cygport 2/3] Don't allow SRC_URI or PATCH_URI to depend on ARCH

Jon TURNEY
In reply to this post by Jon TURNEY
The values which SRC_URI and PATCH_URI evaluate to should not change
depending on ARCH, as this will make the source package arch-dependent
---
 bin/cygport.in | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/bin/cygport.in b/bin/cygport.in
index 6cf0122..25d7e94 100644
--- a/bin/cygport.in
+++ b/bin/cygport.in
@@ -409,6 +409,32 @@ then
  error "${cygportfile} not found.";
 fi
 
+### perform some validation on the .cygport
+
+# SRC_URI and PATCH_URI should not change depending on ARCH, as this will make
+# the source package arch-dependent
+declare -i n
+declare -a VALUE
+ARCHES=("i686" "x86_64" "noarch")
+for VAR in "SRC_URI" "PATCH_URI"
+do
+    n=0
+    while (( n < ${#ARCHES[*]} ))
+    do
+        read -r < <(declare ARCH=${ARCHES[$n]}; declare ARCH_${ARCH}=1; source ${top}/${cygportfile}; eval echo "\$${VAR}")
+        VALUE[$n]=${REPLY}
+        if (( n > 0 ))
+        then
+            if [ "x${VALUE[0]}" != "x${VALUE[$n]}" ]
+            then
+                error "${VAR} appears to depend on ARCH"
+            fi
+        fi
+        n+=1;
+    done
+done
+unset n VALUE ARCHES VAR
+
 ### load .cygport
 source ${top}/${cygportfile} || error "could not read ${cygportfile}"
 ###
--
2.12.2

Reply | Threaded
Open this post in threaded view
|

[PATCH cygport 3/3] Update documentation of all and almostall

Jon TURNEY
In reply to this post by Jon TURNEY
'all of the above' is no longer accurate since the addition of upload etc.
---
 README           | 4 ++--
 lib/help.cygpart | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/README b/README
index 573276e..fa09363 100644
--- a/README
+++ b/README
@@ -146,8 +146,8 @@ to be processed.  All other arguments are interpreted as a COMMAND, which may be
     package   - create binary and source packages
     upload    - upload finished packages to cygwin.com
     finish    - delete the working directory
-    all       - run all of the above, including finish
-    almostall - run all of the above, except for finish
+    all       - run prep, compile, install, package and finish
+    almostall - as all, excluding finish
 
 Other COMMANDs are meant primarily for maintainers:
 
diff --git a/lib/help.cygpart b/lib/help.cygpart
index 6e167ac..073486c 100644
--- a/lib/help.cygpart
+++ b/lib/help.cygpart
@@ -49,7 +49,8 @@ __show_help() {
   upload       upload finished packages to cygwin.com
   announce     send an announcement email to cygwin.com
   finish       delete the working directory
-  all          run all of the above, excluding finish
+  all          run prep, compile, install, package and finish
+  almostall    as all, excluding finish
 
  See the included README file for further documentation.
 
--
2.12.2

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH cygport 2/3] Don't allow SRC_URI or PATCH_URI to depend on ARCH

Andrew Schulman
In reply to this post by Jon TURNEY
> The values which SRC_URI and PATCH_URI evaluate to should not change
> depending on ARCH, as this will make the source package arch-dependent

In that case what's the right thing to do when we have arch-specific
patches? For example screen has one, for x86_64 only.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH cygport 2/3] Don't allow SRC_URI or PATCH_URI to depend on ARCH

Jon TURNEY
On 02/05/2017 14:31, Andrew Schulman wrote:
>> The values which SRC_URI and PATCH_URI evaluate to should not change
>> depending on ARCH, as this will make the source package arch-dependent
>
> In that case what's the right thing to do when we have arch-specific
> patches? For example screen has one, for x86_64 only.

You could wrap C code in the patch in #ifdef __x86_64__ / #endif

However, looking at screen-terminfo-autoconf.patch, it would seem to be
entirely benign to use that on x86.

I don't quite understand how screen reliably builds on any x86_64
platform without that patch...

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH cygport 3/3] Update documentation of all and almostall

Andrew Schulman
In reply to this post by Jon TURNEY
> 'all of the above' is no longer accurate since the addition of upload etc.

In fact, all and almostall are identical. Neither includes finish. See line
627 of /usr/bin/cygport.

Both are misnamed IMO since they exclude get and upload.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH cygport 2/3] Don't allow SRC_URI or PATCH_URI to depend on ARCH

Andrew Schulman
In reply to this post by Jon TURNEY
> On 02/05/2017 14:31, Andrew Schulman wrote:
> >> The values which SRC_URI and PATCH_URI evaluate to should not change
> >> depending on ARCH, as this will make the source package arch-dependent
> >
> > In that case what's the right thing to do when we have arch-specific
> > patches? For example screen has one, for x86_64 only.
>
> You could wrap C code in the patch in #ifdef __x86_64__ / #endif
>
> However, looking at screen-terminfo-autoconf.patch, it would seem to be
> entirely benign to use that on x86.

True. OK, I'll rerelease as 4.5.1-2 with the same source in both arches.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH cygport 3/3] Update documentation of all and almostall

Jon TURNEY
In reply to this post by Andrew Schulman
On 02/05/2017 15:29, Andrew Schulman wrote:
>> 'all of the above' is no longer accurate since the addition of upload etc.
>
> In fact, all and almostall are identical. Neither includes finish. See line
> 627 of /usr/bin/cygport.
>
> Both are misnamed IMO since they exclude get and upload.

Oh wow, I'd completely missed that change [1].  Just think of all the
time I could have saved typing 'all' rather than 'almostall' :-)

Revised patch attached.

[1]
https://github.com/cygwinports/cygport/commit/cf200fb6328323bb3ad5a3d557e11ab7f4203d0c

0003-Update-documentation-of-all.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH cygport 1/3] Pass DEPEND through to .hint for source package as build-depends:

Yaakov Selkowitz
In reply to this post by Jon TURNEY
On 2017-05-02 08:13, Jon Turney wrote:
> Converting a dependency atom to a package name with full generality requires
> a database of all the pathnames contained in all packages, so we don't even
> try to do that here, leaving that task for downstream processing of the
> .hint file...

Or should we just replace DEPEND, which allowed atoms which are
unsupported by setup and whose name is confusing wrt REQUIRES, with a
BUILDREQUIRES variable which is package-name only?

--
Yaakov
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH cygport 3/3] Update documentation of all and almostall

Yaakov Selkowitz
In reply to this post by Jon TURNEY
On 2017-05-02 10:12, Jon Turney wrote:
> Oh wow, I'd completely missed that change [1].  Just think of all the
> time I could have saved typing 'all' rather than 'almostall' :-)

Sorry. :-S

> Revised patch attached.

Merged.  Thanks,

--
Yaakov
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH cygport 2/3] Don't allow SRC_URI or PATCH_URI to depend on ARCH

Yaakov Selkowitz
In reply to this post by Jon TURNEY
On 2017-05-02 08:13, Jon Turney wrote:
> The values which SRC_URI and PATCH_URI evaluate to should not change
> depending on ARCH, as this will make the source package arch-dependent

I think this would require a proper src_prep first, so that patches may
be still be arch-conditionalized there instead.

--
Yaakov
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH cygport 1/3] Pass DEPEND through to .hint for source package as build-depends:

Brian Inglis
In reply to this post by Yaakov Selkowitz
On 2017-05-05 14:42, Yaakov Selkowitz wrote:
> On 2017-05-02 08:13, Jon Turney wrote:
>> Converting a dependency atom to a package name with full generality
>> requires a database of all the pathnames contained in all packages,
>> so we don't even try to do that here, leaving that task for
>> downstream processing of the .hint file...
> Or should we just replace DEPEND, which allowed atoms which are
> unsupported by setup and whose name is confusing wrt REQUIRES, with a
> BUILDREQUIRES variable which is package-name only?

Confusion could be handled by adding DEPENDS contents to DEPEND
variables after including pkg.cygport.
The current DEPEND atoms are useful for packaged modules, but that
does not preclude specifying just a pkg name, and maintainers could
specify either a module or pkg.
Would it be useful to resolve DEPEND to pkg names and add
"build-requires" or "build-depends" in downstream processing?

--
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH cygport 1/3] Pass DEPEND through to .hint for source package as build-depends:

Achim Gratz
In reply to this post by Yaakov Selkowitz
Yaakov Selkowitz writes:
> Or should we just replace DEPEND, which allowed atoms which are
> unsupported by setup and whose name is confusing wrt REQUIRES, with a
> BUILDREQUIRES variable which is package-name only?

BUILDREQUIRES sounds OK to me and somewhat more in line with other
packaging systems.


Regards,
Achim.
--
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf microQ V2.22R2:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH cygport 1/3] Pass DEPEND through to .hint for source package as build-depends:

Jon TURNEY
In reply to this post by Yaakov Selkowitz
On 05/05/2017 21:42, Yaakov Selkowitz wrote:

> On 2017-05-02 08:13, Jon Turney wrote:
>> Converting a dependency atom to a package name with full generality
>> requires
>> a database of all the pathnames contained in all packages, so we don't
>> even
>> try to do that here, leaving that task for downstream processing of the
>> .hint file...
>
> Or should we just replace DEPEND, which allowed atoms which are
> unsupported by setup and whose name is confusing wrt REQUIRES, with a
> BUILDREQUIRES variable which is package-name only?

Changing the name to something less confusing seems like a good idea.

I kind of like the current scheme, as it can do some checks when
cross-compiling as well.

I think that something (calm?) needs to keep a database of all package
pathnames, to check for pathname collisions between packages (I suspect
we have some unnoticed cases of that at the moment), so the extra cost
of doing a conversion from cygport dependency atoms in a .hint to
package names in setup.ini shouldn't be too much.  It'll be a while
before that happens, though... :)

People also like to typo this as DEPENDS, so some sort of warning that
the .cygport sets a variable which has no effect might also be useful.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH cygport 2/3] Don't allow SRC_URI or PATCH_URI to depend on ARCH

Jon TURNEY
In reply to this post by Yaakov Selkowitz
On 05/05/2017 23:06, Yaakov Selkowitz wrote:
> On 2017-05-02 08:13, Jon Turney wrote:
>> The values which SRC_URI and PATCH_URI evaluate to should not change
>> depending on ARCH, as this will make the source package arch-dependent
>
> I think this would require a proper src_prep first, so that patches may
> be still be arch-conditionalized there instead.

Ok, that makes sense.

So I guess there could be a hook called for each patch to cause it to be
applied/skipped, or to adjust the patch list?  Or do you think more of
__src_prep needs to be customizable?

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH cygport 1/3] Pass DEPEND through to .hint for source package as build-depends:

Brian Inglis
In reply to this post by Jon TURNEY
On 2017-05-06 05:03, Jon Turney wrote:

> On 05/05/2017 21:42, Yaakov Selkowitz wrote:
>> On 2017-05-02 08:13, Jon Turney wrote:
>>> Converting a dependency atom to a package name with full generality
>>> requires
>>> a database of all the pathnames contained in all packages, so we don't
>>> even
>>> try to do that here, leaving that task for downstream processing of the
>>> .hint file...
>> Or should we just replace DEPEND, which allowed atoms which are
>> unsupported by setup and whose name is confusing wrt REQUIRES, with a
>> BUILDREQUIRES variable which is package-name only?
> Changing the name to something less confusing seems like a good
> idea.
> I kind of like the current scheme, as it can do some checks when
> cross-compiling as well.
> I think that something (calm?) needs to keep a database of all
> package pathnames, to check for pathname collisions between packages
> (I suspect we have some unnoticed cases of that at the moment), so
> the extra cost of doing a conversion from cygport dependency atoms in
> a .hint to package names in setup.ini shouldn't be too much. It'll be
> a while before that happens, though... :)
> People also like to typo this as DEPENDS, so some sort of warning
> that the .cygport sets a variable which has no effect might also be
> useful.

Append to DEPEND with a warning?

--
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH cygport 2/3] Don't allow SRC_URI or PATCH_URI to depend on ARCH

Jon TURNEY
In reply to this post by Jon TURNEY
On 06/05/2017 12:32, Jon Turney wrote:

> On 05/05/2017 23:06, Yaakov Selkowitz wrote:
>> On 2017-05-02 08:13, Jon Turney wrote:
>>> The values which SRC_URI and PATCH_URI evaluate to should not change
>>> depending on ARCH, as this will make the source package arch-dependent
>>
>> I think this would require a proper src_prep first, so that patches may
>> be still be arch-conditionalized there instead.
>
> Ok, that makes sense.
>
> So I guess there could be a hook called for each patch to cause it to be
> applied/skipped, or to adjust the patch list?  Or do you think more of
> __src_prep needs to be customizable?

Attached is an implementation of the first suggestion.



0001-Add-src_patch_apply_hook.patch (2K) Download Attachment