build: Don't use non-posix arguments for build without GNU coreutils
diff mbox series

Message ID 20240606130102.2660-2-zachdecook@librem.one
State New
Headers show
Series
  • build: Don't use non-posix arguments for build without GNU coreutils
Related show

Commit Message

Zach DeCook June 6, 2024, 1:01 p.m. UTC
date will output a date like
2024-06-05T17:06:30EDT

(time zone is different than currently specified)

ln won't create a relative link
(not a big deal because the command gets reran each time)
---
specifically tested in Alpine Linux, using busybox utils
 src/py/libcamera/meson.build | 4 ++--
 utils/gen-version.sh         | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Laurent Pinchart June 11, 2024, 10:22 p.m. UTC | #1
Hi Zach,

Thank you for the patch.

On Thu, Jun 06, 2024 at 09:01:03AM -0400, Zach DeCook wrote:
> date will output a date like
> 2024-06-05T17:06:30EDT

Looks like posix doesn't define a %z :( I think this one is probably OK.
Kieran, any opinion ?

Where do the date and ln tools come from in your build environment ?

> 
> (time zone is different than currently specified)
> 
> ln won't create a relative link
> (not a big deal because the command gets reran each time)

Won't that cause issues if the directories are exported over NFS and
mounted on a different absolute path ? I think the point of the Python
links was to support cross-compiling on a host and running on a target
device with an NFS mount. Tomi, is this correct ?

> ---
> specifically tested in Alpine Linux, using busybox utils
>  src/py/libcamera/meson.build | 4 ++--
>  utils/gen-version.sh         | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build
> index 4807ca7d..524d010c 100644
> --- a/src/py/libcamera/meson.build
> +++ b/src/py/libcamera/meson.build
> @@ -98,11 +98,11 @@ pycamera = shared_module('_libcamera',
>  # Create symlinks from the build dir to the source dir so that we can use the
>  # Python module directly from the build dir.
>  
> -run_command('ln', '-fsrT', files('__init__.py'),
> +run_command('ln', '-fs', files('__init__.py'),
>              meson.current_build_dir() / '__init__.py',
>              check : true)
>  
> -run_command('ln', '-fsrT', meson.current_source_dir() / 'utils',
> +run_command('ln', '-fs', meson.current_source_dir() / 'utils',
>              meson.current_build_dir() / 'utils',
>              check : true)
>  
> diff --git a/utils/gen-version.sh b/utils/gen-version.sh
> index e1f7ca7b..1b818e9e 100755
> --- a/utils/gen-version.sh
> +++ b/utils/gen-version.sh
> @@ -42,7 +42,7 @@ if [ -z "$build_dir" ] || (echo "$build_dir" | grep -q "$src_dir")
>  then
>  	git update-index --refresh > /dev/null 2>&1
>  fi
> -git diff-index --quiet HEAD || version="$version-dirty ($(date --iso-8601=seconds))"
> +git diff-index --quiet HEAD || version="$version-dirty ($(date +%Y-%m-%dT%H:%M:%S%Z))"
>  
>  # If a project version is provided, use it to replace the version number.
>  if [ -n "$project_version" ]
Tomi Valkeinen June 12, 2024, 5:54 a.m. UTC | #2
On 12/06/2024 01:22, Laurent Pinchart wrote:
> Hi Zach,
> 
> Thank you for the patch.
> 
> On Thu, Jun 06, 2024 at 09:01:03AM -0400, Zach DeCook wrote:
>> date will output a date like
>> 2024-06-05T17:06:30EDT
> 
> Looks like posix doesn't define a %z :( I think this one is probably OK.
> Kieran, any opinion ?
> 
> Where do the date and ln tools come from in your build environment ?
> 
>>
>> (time zone is different than currently specified)
>>
>> ln won't create a relative link
>> (not a big deal because the command gets reran each time)
> 
> Won't that cause issues if the directories are exported over NFS and
> mounted on a different absolute path ? I think the point of the Python
> links was to support cross-compiling on a host and running on a target
> device with an NFS mount. Tomi, is this correct ?

Yes. Absolute links are not nice...

  Tomi

>> ---
>> specifically tested in Alpine Linux, using busybox utils
>>   src/py/libcamera/meson.build | 4 ++--
>>   utils/gen-version.sh         | 2 +-
>>   2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build
>> index 4807ca7d..524d010c 100644
>> --- a/src/py/libcamera/meson.build
>> +++ b/src/py/libcamera/meson.build
>> @@ -98,11 +98,11 @@ pycamera = shared_module('_libcamera',
>>   # Create symlinks from the build dir to the source dir so that we can use the
>>   # Python module directly from the build dir.
>>   
>> -run_command('ln', '-fsrT', files('__init__.py'),
>> +run_command('ln', '-fs', files('__init__.py'),
>>               meson.current_build_dir() / '__init__.py',
>>               check : true)
>>   
>> -run_command('ln', '-fsrT', meson.current_source_dir() / 'utils',
>> +run_command('ln', '-fs', meson.current_source_dir() / 'utils',
>>               meson.current_build_dir() / 'utils',
>>               check : true)
>>   
>> diff --git a/utils/gen-version.sh b/utils/gen-version.sh
>> index e1f7ca7b..1b818e9e 100755
>> --- a/utils/gen-version.sh
>> +++ b/utils/gen-version.sh
>> @@ -42,7 +42,7 @@ if [ -z "$build_dir" ] || (echo "$build_dir" | grep -q "$src_dir")
>>   then
>>   	git update-index --refresh > /dev/null 2>&1
>>   fi
>> -git diff-index --quiet HEAD || version="$version-dirty ($(date --iso-8601=seconds))"
>> +git diff-index --quiet HEAD || version="$version-dirty ($(date +%Y-%m-%dT%H:%M:%S%Z))"
>>   
>>   # If a project version is provided, use it to replace the version number.
>>   if [ -n "$project_version" ]
>
Kieran Bingham June 12, 2024, 9:41 a.m. UTC | #3
Quoting Tomi Valkeinen (2024-06-12 06:54:57)
> On 12/06/2024 01:22, Laurent Pinchart wrote:
> > Hi Zach,
> > 
> > Thank you for the patch.
> > 
> > On Thu, Jun 06, 2024 at 09:01:03AM -0400, Zach DeCook wrote:
> >> date will output a date like
> >> 2024-06-05T17:06:30EDT
> > 
> > Looks like posix doesn't define a %z :( I think this one is probably OK.
> > Kieran, any opinion ?
> > 
> > Where do the date and ln tools come from in your build environment ?

alpine+busybox seems to support %z (though not %:z which is a more exact
match)

kbingham@Monstersaurus:~$ run-in-docker alpine
Executing in alpine
/home/kbingham # date +"%Y-%m-%dT%H:%M:%S%:z"
2024-06-12T08:56:32
/home/kbingham # busybox date +"%Y-%m-%dT%H:%M:%S%:z"
2024-06-12T08:57:51
/home/kbingham # busybox date +"%Y-%m-%dT%H:%M:%S"
2024-06-12T08:58:01
/home/kbingham # busybox date +"%Y-%m-%dT%H:%M:%S%z"
2024-06-12T08:58:04+0000
/home/kbingham # busybox date +"%Y-%m-%dT%H:%M:%S%Z"
2024-06-12T08:58:08UTC


So if this is just about fixing/supporting alpine builds I'd go for the
%z as it will sort better in the event the version string gets used to
actually sort between two competing versions built at the same time but
in different timezones ... but we're splitting hairs here so I don't
care ;-)



> >> (time zone is different than currently specified)
> >>
> >> ln won't create a relative link
> >> (not a big deal because the command gets reran each time)
> > 
> > Won't that cause issues if the directories are exported over NFS and
> > mounted on a different absolute path ? I think the point of the Python
> > links was to support cross-compiling on a host and running on a target
> > device with an NFS mount. Tomi, is this correct ?
> 
> Yes. Absolute links are not nice...
> 
>   Tomi
> 
> >> ---
> >> specifically tested in Alpine Linux, using busybox utils
> >>   src/py/libcamera/meson.build | 4 ++--
> >>   utils/gen-version.sh         | 2 +-
> >>   2 files changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build
> >> index 4807ca7d..524d010c 100644
> >> --- a/src/py/libcamera/meson.build
> >> +++ b/src/py/libcamera/meson.build
> >> @@ -98,11 +98,11 @@ pycamera = shared_module('_libcamera',
> >>   # Create symlinks from the build dir to the source dir so that we can use the
> >>   # Python module directly from the build dir.
> >>   
> >> -run_command('ln', '-fsrT', files('__init__.py'),
> >> +run_command('ln', '-fs', files('__init__.py'),
> >>               meson.current_build_dir() / '__init__.py',
> >>               check : true)

Busybox supports -T "Treat LINK as a file, not DIR" does that need to be
dropped? (at least that sounds compatible against "-T,
--no-target-directory   treat LINK_NAME as a normal file always")

I do worry though that the 'benefit' of these symlinks is being
completley lost if the relative paths aren't maintained though.

Seems like it would negate the whole point of the symlink being there at
all maybe if it couldn't actually be used by the implementor...

So if this patch was split, I'd be happy to merge the date fix already -
but the symlink issue sounds like it needs more investigation?

Presumably we'll have to have a fallback command or have the command not
run at all - as the case we're trying to fix here probably aren't going
to run the python scripts locally in the build... which is what I
believe this symlink supports.

So probably better to make the link conditional on only creating it if
it's possible.



> >>   
> >> -run_command('ln', '-fsrT', meson.current_source_dir() / 'utils',
> >> +run_command('ln', '-fs', meson.current_source_dir() / 'utils',
> >>               meson.current_build_dir() / 'utils',
> >>               check : true)
> >>   
> >> diff --git a/utils/gen-version.sh b/utils/gen-version.sh
> >> index e1f7ca7b..1b818e9e 100755
> >> --- a/utils/gen-version.sh
> >> +++ b/utils/gen-version.sh
> >> @@ -42,7 +42,7 @@ if [ -z "$build_dir" ] || (echo "$build_dir" | grep -q "$src_dir")
> >>   then
> >>      git update-index --refresh > /dev/null 2>&1
> >>   fi
> >> -git diff-index --quiet HEAD || version="$version-dirty ($(date --iso-8601=seconds))"
> >> +git diff-index --quiet HEAD || version="$version-dirty ($(date +%Y-%m-%dT%H:%M:%S%Z))"
> >>   
> >>   # If a project version is provided, use it to replace the version number.
> >>   if [ -n "$project_version" ]
> > 
>

Patch
diff mbox series

diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build
index 4807ca7d..524d010c 100644
--- a/src/py/libcamera/meson.build
+++ b/src/py/libcamera/meson.build
@@ -98,11 +98,11 @@  pycamera = shared_module('_libcamera',
 # Create symlinks from the build dir to the source dir so that we can use the
 # Python module directly from the build dir.
 
-run_command('ln', '-fsrT', files('__init__.py'),
+run_command('ln', '-fs', files('__init__.py'),
             meson.current_build_dir() / '__init__.py',
             check : true)
 
-run_command('ln', '-fsrT', meson.current_source_dir() / 'utils',
+run_command('ln', '-fs', meson.current_source_dir() / 'utils',
             meson.current_build_dir() / 'utils',
             check : true)
 
diff --git a/utils/gen-version.sh b/utils/gen-version.sh
index e1f7ca7b..1b818e9e 100755
--- a/utils/gen-version.sh
+++ b/utils/gen-version.sh
@@ -42,7 +42,7 @@  if [ -z "$build_dir" ] || (echo "$build_dir" | grep -q "$src_dir")
 then
 	git update-index --refresh > /dev/null 2>&1
 fi
-git diff-index --quiet HEAD || version="$version-dirty ($(date --iso-8601=seconds))"
+git diff-index --quiet HEAD || version="$version-dirty ($(date +%Y-%m-%dT%H:%M:%S%Z))"
 
 # If a project version is provided, use it to replace the version number.
 if [ -n "$project_version" ]