Patchwork tzdata: We shouldn't override the localtime if it is valid

login
register
mail settings
Submitter Otavio Salvador
Date Feb. 8, 2013, 11:32 a.m.
Message ID <1360323133-28592-1-git-send-email-otavio@ossystems.com.br>
Download mbox | patch
Permalink /patch/44329/
State Accepted
Commit a9b42c8e85332a65788b1434f926186d4887b287
Headers show

Comments

Otavio Salvador - Feb. 8, 2013, 11:32 a.m.
The code where mistakenly replacing the localtime file setting so we
end with a copy of file instead of a symbolic link. This fixes it so
now, we'll only do that in case the link is pointing to invalid data.

Change-Id: I16dfa5ea4f293c48bb396f4e23a2ea53e6c9e745
Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
---
 meta/recipes-extended/tzdata/tzdata_2012d.bb |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)
Ross Burton - Feb. 8, 2013, 11:36 a.m.
On 8 February 2013 11:32, Otavio Salvador <otavio@ossystems.com.br> wrote:
> The code where mistakenly replacing the localtime file setting so we
> end with a copy of file instead of a symbolic link. This fixes it so
> now, we'll only do that in case the link is pointing to invalid data.

Can you fix the code to symlink instead of copy?  systemd and more
needs a symlink so that it can identify the name of the timezone from
the link target.

Ross
Otavio Salvador - Feb. 8, 2013, 11:38 a.m.
On Fri, Feb 8, 2013 at 9:36 AM, Burton, Ross <ross.burton@intel.com> wrote:
> On 8 February 2013 11:32, Otavio Salvador <otavio@ossystems.com.br> wrote:
>> The code where mistakenly replacing the localtime file setting so we
>> end with a copy of file instead of a symbolic link. This fixes it so
>> now, we'll only do that in case the link is pointing to invalid data.
>
> Can you fix the code to symlink instead of copy?  systemd and more
> needs a symlink so that it can identify the name of the timezone from
> the link target.

You mean inside the if?
Ross Burton - Feb. 8, 2013, 11:55 a.m.
On 8 February 2013 11:38, Otavio Salvador <otavio@ossystems.com.br> wrote:
>> Can you fix the code to symlink instead of copy?  systemd and more
>> needs a symlink so that it can identify the name of the timezone from
>> the link target.
>
> You mean inside the if?

I mean in general /etc/localtime should be a symlink and not a copy.

Ross
Otavio Salvador - Feb. 8, 2013, 11:59 a.m.
On Fri, Feb 8, 2013 at 9:55 AM, Burton, Ross <ross.burton@intel.com> wrote:
> On 8 February 2013 11:38, Otavio Salvador <otavio@ossystems.com.br> wrote:
>>> Can you fix the code to symlink instead of copy?  systemd and more
>>> needs a symlink so that it can identify the name of the timezone from
>>> the link target.
>>
>> You mean inside the if?
>
> I mean in general /etc/localtime should be a symlink and not a copy.

Ok; but I will do it in another patch ... in fact it could be
simplified a lot. I will propose a patch for it.
Tomas Frydrych - Feb. 8, 2013, noon
On 08/02/13 11:55, Burton, Ross wrote:
> On 8 February 2013 11:38, Otavio Salvador <otavio@ossystems.com.br> wrote:
>>> Can you fix the code to symlink instead of copy?  systemd and more
>>> needs a symlink so that it can identify the name of the timezone from
>>> the link target.
>>
>> You mean inside the if?
> 
> I mean in general /etc/localtime should be a symlink and not a copy.

/etc/localtime is a real file so that /etc is not containing a link to a
file in /usr; this has historically been the case unless I am not
mistaken. systemd should grow up and read the contents :).

Tomas
Otavio Salvador - Feb. 8, 2013, 12:07 p.m.
On Fri, Feb 8, 2013 at 10:00 AM, Tomas Frydrych
<tf+lists.yocto@r-finger.com> wrote:
> On 08/02/13 11:55, Burton, Ross wrote:
>> On 8 February 2013 11:38, Otavio Salvador <otavio@ossystems.com.br> wrote:
>>>> Can you fix the code to symlink instead of copy?  systemd and more
>>>> needs a symlink so that it can identify the name of the timezone from
>>>> the link target.
>>>
>>> You mean inside the if?
>>
>> I mean in general /etc/localtime should be a symlink and not a copy.
>
> /etc/localtime is a real file so that /etc is not containing a link to a
> file in /usr; this has historically been the case unless I am not
> mistaken. systemd should grow up and read the contents :).

In fact a lot of software expects it to be a symlink so it shouldn't be a copy.
Ross Burton - Feb. 8, 2013, 12:11 p.m.
On 8 February 2013 12:00, Tomas Frydrych <tf+lists.yocto@r-finger.com> wrote:
> /etc/localtime is a real file so that /etc is not containing a link to a
> file in /usr; this has historically been the case unless I am not
> mistaken. systemd should grow up and read the contents :).

But the contents don't identify the name of the timezone.

Ross
Tomas Frydrych - Feb. 8, 2013, 12:27 p.m.
On 08/02/13 12:07, Otavio Salvador wrote:
> On Fri, Feb 8, 2013 at 10:00 AM, Tomas Frydrych
> <tf+lists.yocto@r-finger.com> wrote:
>> On 08/02/13 11:55, Burton, Ross wrote:
>>> On 8 February 2013 11:38, Otavio Salvador <otavio@ossystems.com.br> wrote:
>>>>> Can you fix the code to symlink instead of copy?  systemd and more
>>>>> needs a symlink so that it can identify the name of the timezone from
>>>>> the link target.
>>>>
>>>> You mean inside the if?
>>>
>>> I mean in general /etc/localtime should be a symlink and not a copy.
>>
>> /etc/localtime is a real file so that /etc is not containing a link to a
>> file in /usr; this has historically been the case unless I am not
>> mistaken. systemd should grow up and read the contents :).
> 
> In fact a lot of software expects it to be a symlink so it shouldn't be a copy.

Lot of software does thing without understanding what it is doing,
that's the nature of software. :) My point is that the code in the
current recipe is not doing the unconditional copy by mistake, it is
doing so intentionally so that local clock can work without /usr. Only
systems that mandate the presence of /usr can use a symlink -- so this
is not a bug fix but a change in an established system behaviour. If
nothing else, the commit message should be clear in this regard.

Tomas
Otavio Salvador - Feb. 8, 2013, 12:36 p.m.
On Fri, Feb 8, 2013 at 10:27 AM, Tomas Frydrych
<tf+lists.yocto@r-finger.com> wrote:
> On 08/02/13 12:07, Otavio Salvador wrote:
>> On Fri, Feb 8, 2013 at 10:00 AM, Tomas Frydrych
>> <tf+lists.yocto@r-finger.com> wrote:
>>> On 08/02/13 11:55, Burton, Ross wrote:
>>>> On 8 February 2013 11:38, Otavio Salvador <otavio@ossystems.com.br> wrote:
>>>>>> Can you fix the code to symlink instead of copy?  systemd and more
>>>>>> needs a symlink so that it can identify the name of the timezone from
>>>>>> the link target.
>>>>>
>>>>> You mean inside the if?
>>>>
>>>> I mean in general /etc/localtime should be a symlink and not a copy.
>>>
>>> /etc/localtime is a real file so that /etc is not containing a link to a
>>> file in /usr; this has historically been the case unless I am not
>>> mistaken. systemd should grow up and read the contents :).
>>
>> In fact a lot of software expects it to be a symlink so it shouldn't be a copy.
>
> Lot of software does thing without understanding what it is doing,
> that's the nature of software. :) My point is that the code in the
> current recipe is not doing the unconditional copy by mistake, it is
> doing so intentionally so that local clock can work without /usr. Only
> systems that mandate the presence of /usr can use a symlink -- so this
> is not a bug fix but a change in an established system behaviour. If
> nothing else, the commit message should be clear in this regard.

I agree about the commit log. I'd for ignore the possibility to use
/etc without /usr as the applications running at very beggning will
start using GMT and move to timezone when /usr is ready. I see no
problem with that.

Ross, we are at same page? If yes I will send a v3 with improved commit log.
Ross Burton - Feb. 8, 2013, 2:43 p.m.
On 8 February 2013 12:36, Otavio Salvador <otavio@ossystems.com.br> wrote:
> I agree about the commit log. I'd for ignore the possibility to use
> /etc without /usr as the applications running at very beggning will
> start using GMT and move to timezone when /usr is ready. I see no
> problem with that.
>
> Ross, we are at same page? If yes I will send a v3 with improved commit log.

Unless someone has a good argument against this, I agree with you.

Ross

Patch

diff --git a/meta/recipes-extended/tzdata/tzdata_2012d.bb b/meta/recipes-extended/tzdata/tzdata_2012d.bb
index 9741101..9ec6715 100644
--- a/meta/recipes-extended/tzdata/tzdata_2012d.bb
+++ b/meta/recipes-extended/tzdata/tzdata_2012d.bb
@@ -5,7 +5,7 @@  LICENSE = "PD"
 LIC_FILES_CHKSUM = "file://asia;beginline=2;endline=3;md5=06468c0e84ef4d4c97045a4a29b08234"
 DEPENDS = "tzcode-native"
 
-PR = "r2"
+PR = "r3"
 
 inherit allarch
 
@@ -93,12 +93,12 @@  pkg_postinst_${PN} () {
 		echo "You have an invalid TIMEZONE setting in ${src}"
 		echo "Your ${etc_lt} has been reset to Universal; enjoy!"
 		tz="Universal"
+		echo "Updating ${etc_lt} with $D${datadir}/zoneinfo/${tz}"
+		if [ -L ${etc_lt} ] ; then
+			rm -f "${etc_lt}"
+		fi
+		cp -f "$D${datadir}/zoneinfo/${tz}" "${etc_lt}"
 	fi
-	echo "Updating ${etc_lt} with $D${datadir}/zoneinfo/${tz}"
-	if [ -L ${etc_lt} ] ; then
-		rm -f "${etc_lt}"
-	fi
-	cp -f "$D${datadir}/zoneinfo/${tz}" "${etc_lt}"
 }
 
 # Packages primarily organized by directory with a major city