Re: setting timestamps
At 11:30 PM 7/8/2003, William A. Rowe, Jr. wrote: >The general suggestion is goodness, but the patch below would >continue to promote ctime ambiguity. I'm sorry, now I see this isn't the set-mtime-ctime-atime patch, but sets the mtime member discretely. That would be fine for all platforms. Of course we add each supported time stamp per-platform, so win32 gets a crtime_set, and unix an itime_set (or perhaps imtime, e.g. the inode modified time?) Semantics aside, this patch did look like a good idea. Bill
Re: setting timestamps
Guys... the API is wrong, as we've been scolded about several times.
In APR 1.0, we've discussed eliminating ctime (ambigious) for some
other more specific datum (e.g. itime for the inode-info time modification
on unix, and crtime for the file creation time on Win32.)
The general suggestion is goodness, but the patch below would
continue to promote ctime ambiguity.
Bill
At 10:28 PM 7/2/2003, Cliff Woolley wrote:
>On Wed, 2 Jul 2003, Ben Collins-Sussman wrote:
>
>> Subversion is already using apr_stat() to read the mtime of a file.
>> But now we'd like to be able to *write* timestamps as well.
>> Has this conversation been had before?
>
>I did some digging on marc.theaimsgroup and found this:
>
>http://marc.theaimsgroup.com/?l=apr-dev&m=104970018703214&w=2
>
>It looks like a patch was submitted to do exactly that by Matt Kraai,
>though it only provided an implementation for unix, and it seems to have
>fallen through the cracks anyway. Justin reviewed the original patch,
>though the revised one seems to have gotten no response. It was:
>
>Index: file_io/unix/filestat.c
>===
>RCS file: /home/cvspublic/apr/file_io/unix/filestat.c,v
>retrieving revision 1.65
>diff -u -r1.65 filestat.c
>--- file_io/unix/filestat.c 6 Mar 2003 09:21:24 - 1.65
>+++ file_io/unix/filestat.c 4 Apr 2003 03:58:25 -
>@@ -208,6 +208,30 @@
> return apr_file_perms_set(fname, finfo.protection);
> }
>
>+APR_DECLARE(apr_status_t) apr_file_mtime_set(const char *fname,
>+ apr_time_t mtime,
>+ apr_pool_t *pool)
>+{
>+apr_status_t status;
>+apr_finfo_t finfo;
>+struct timeval tvp[2];
>+
>+status = apr_stat(&finfo, fname, APR_FINFO_ATIME, pool);
>+if (!APR_STATUS_IS_SUCCESS(status)) {
>+return status;
>+}
>+
>+tvp[0].tv_sec = apr_time_sec(finfo.atime);
>+tvp[0].tv_usec = apr_time_usec(finfo.atime);
>+tvp[1].tv_sec = apr_time_sec(mtime);
>+tvp[1].tv_usec = apr_time_usec(mtime);
>+
>+if (utimes(fname, tvp) == -1) {
>+return errno;
>+}
>+return APR_SUCCESS;
>+}
>+
> APR_DECLARE(apr_status_t) apr_stat(apr_finfo_t *finfo,
>const char *fname,
>apr_int32_t wanted, apr_pool_t *pool)
>Index: include/apr_file_io.h
>===
>RCS file: /home/cvspublic/apr/include/apr_file_io.h,v
>retrieving revision 1.138
>diff -u -r1.138 apr_file_io.h
>--- include/apr_file_io.h 3 Apr 2003 23:20:05 - 1.138
>+++ include/apr_file_io.h 4 Apr 2003 03:58:27 -
>@@ -658,6 +658,18 @@
> apr_pool_t *cont);
>
> /**
>+ * Set the mtime of the specified file.
>+ * @param fname The full path to the file (using / on all systems)
>+ * @param mtime The mtime to apply to the file.
>+ * @param pool The pool to use.
>+ * @warning Platforms which do not implement this feature will return
>+ * APR_ENOTIMPL.
>+ */
>+APR_DECLARE(apr_status_t) apr_file_mtime_set(const char *fname,
>+ apr_time_t mtime,
>+ apr_pool_t *pool);
>+
>+/**
> * Create a new directory on the file system.
> * @param path the path for the directory to be created. (use / on all
> systems)
> * @param perm Permissions for the new direcoty.
Re: setting timestamps
On Tue, 8 Jul 2003, [UTF-8] Branko Ä^Libej wrote: > And a symbol that's not defined is treated as if it were zero by the C > preprocessor. So it comes to the same thing in the end. :-) I know it is on gcc, but I didn't know if you could count on that being the case portably. If so, great! :) --Cliff
Re: setting timestamps
Cliff Woolley wrote: >On Tue, 8 Jul 2003, [UTF-8] Branko ?^Libej wrote: > > > >>on those defines, giving utimes (which has the more precise interface) >>priority; e.g., >> >>#if HAVE_UTIMES >>...use utimes... >>#elif HAVE_UTIME >> >> > >IIRC, that should be: > >#ifdef HAVE_UTIMES >...use utimes... >#elif defined(HAVE_UTIME) > >It would be #if if it were an APR_HAVE_ macro, which are defined always >but have a value of zero or one depending. The ones that comes straight >out of autoconf are either defined or not. > And a symbol that's not defined is treated as if it were zero by the C preprocessor. So it comes to the same thing in the end. :-) -- Brane Äibej <[EMAIL PROTECTED]> http://www.xbc.nu/brane/
Re: setting timestamps
On Tue, 8 Jul 2003, Ben Collins-Sussman wrote: > Um, actually, apr_private.h.in isn't under version control. It's > auto-generated by 'autoheader', which reads configure.in. That stuff > automatically appeared when I added AC_CHECK_FUNCS(utimes utime). Oops. :)
Re: setting timestamps
Cliff Woolley <[EMAIL PROTECTED]> writes: > You also have to edit include/arch/unix/apr_private.h.in and add a set of > lines like: > > /* define this if your system has the utimes() function */ > #undef HAVE_UTIMES > > /* define this if your system has the utime() function */ > #undef HAVE_UTIME Um, actually, apr_private.h.in isn't under version control. It's auto-generated by 'autoheader', which reads configure.in. That stuff automatically appeared when I added AC_CHECK_FUNCS(utimes utime).
Re: setting timestamps
On Tue, 8 Jul 2003, [UTF-8] Branko Ä^Libej wrote: > on those defines, giving utimes (which has the more precise interface) > priority; e.g., > > #if HAVE_UTIMES > ...use utimes... > #elif HAVE_UTIME IIRC, that should be: #ifdef HAVE_UTIMES ...use utimes... #elif defined(HAVE_UTIME) It would be #if if it were an APR_HAVE_ macro, which are defined always but have a value of zero or one depending. The ones that comes straight out of autoconf are either defined or not. You also have to edit include/arch/unix/apr_private.h.in and add a set of lines like: /* define this if your system has the utimes() function */ #undef HAVE_UTIMES /* define this if your system has the utime() function */ #undef HAVE_UTIME --Cliff
Re: setting timestamps
Ben Collins-Sussman wrote: >Branko Äibej <[EMAIL PROTECTED]> writes: > > > >>This interface looks sane, and I can cobble up a Windows implementation >>in two eyeblinks. As for the patch itself: utimes() is a BSDism and may >>not be available on all systems. The Unix implementation needs to check >>for this, and use utime() instead if utimes() isn't available. >> >> > >Branko, I just committed your & Matt's patches for >apr_file_mtime_set(). Your test passes for me. > >Can you give me a clue about how to "check for utimes()" on unix >platforms? > > Configury, my man! Find an appropriate place in configure.in, then: AC_CHECK_FUNCS(utime, utimes) That'll get HAVE_UTIME and/or HAVE_UTIMES defined in include/arch/unix/apr_private.h. Then you just condinitonalize the code on those defines, giving utimes (which has the more precise interface) priority; e.g., #if HAVE_UTIMES ...use utimes... #elif HAVE_UTIME ...use utime... #else return APR_ENOTIMPL: #endif Not that there's much chance that any Unix wouldn't have at least utime(). I don't think you have to tweak apr.h.in. There's no reason to export which syscall we're using. You do have to put in dummy implementations that return APR_ENOTIMPL for OS/2 and Netware, though. -- Brane Äibej <[EMAIL PROTECTED]> http://www.xbc.nu/brane/
Re: setting timestamps
Branko Äibej <[EMAIL PROTECTED]> writes: > This interface looks sane, and I can cobble up a Windows implementation > in two eyeblinks. As for the patch itself: utimes() is a BSDism and may > not be available on all systems. The Unix implementation needs to check > for this, and use utime() instead if utimes() isn't available. Branko, I just committed your & Matt's patches for apr_file_mtime_set(). Your test passes for me. Can you give me a clue about how to "check for utimes()" on unix platforms?
Re: setting timestamps
On Fri, 4 Jul 2003, [UTF-8] Branko Ä^Libej wrote: > How? These flags are only used within APR, and are not even visible > outside APR. Oh? My fault, I'm sorry. I misread the fact that they had an APR_ namespace prefix as meaning that they were in the public headers and didn't pay attention to where they really were. +1 in that case. :) Thanks, Cliff
Re: setting timestamps
Cliff Woolley wrote: >On Fri, 4 Jul 2003, [UTF-8] Branko ?^Libej wrote: > > > >> /* Internal Flags for apr_file_open */ >>-#define APR_OPENINFO 0x4000/* Open without READ or WRITE access */ >>-#define APR_OPENLINK 0x2000/* Open a link itself, if supported */ >>-#define APR_READCONTROL 0x1000/* Read the file's owner/perms */ >>-#define APR_WRITECONTROL 0x0800/* Modifythe file's owner/perms */ >>+#define APR_OPENINFO 0x0010 /* Open without READ or WRITE access */ >>+#define APR_OPENLINK 0x0020 /* Open a link itself, if supported */ >>+#define APR_READCONTROL 0x0040 /* Read the file's owner/perms */ >>+#define APR_WRITECONTROL 0x0080 /* Modifythe file's owner/perms */ >>+#define APR_WRITEATTRS 0x0100 /* Modify the file's attributes */ >> >> > >I should point out that this would break binary compatibility... > > How? These flags are only used within APR, and are not even visible outside APR. -- Brane Äibej <[EMAIL PROTECTED]> http://www.xbc.nu/brane/
Re: setting timestamps
On Fri, 4 Jul 2003, [UTF-8] Branko Ä^Libej wrote: > /* Internal Flags for apr_file_open */ > -#define APR_OPENINFO 0x4000/* Open without READ or WRITE access */ > -#define APR_OPENLINK 0x2000/* Open a link itself, if supported */ > -#define APR_READCONTROL 0x1000/* Read the file's owner/perms */ > -#define APR_WRITECONTROL 0x0800/* Modifythe file's owner/perms */ > +#define APR_OPENINFO 0x0010 /* Open without READ or WRITE access */ > +#define APR_OPENLINK 0x0020 /* Open a link itself, if supported */ > +#define APR_READCONTROL 0x0040 /* Read the file's owner/perms */ > +#define APR_WRITECONTROL 0x0080 /* Modifythe file's owner/perms */ > +#define APR_WRITEATTRS 0x0100 /* Modify the file's attributes */ I should point out that this would break binary compatibility... --Cliff
Re: setting timestamps
I wrote:
>This interface looks sane, and I can cobble up a Windows implementation
>in two eyeblinks.
>
>
And here it is, including tests.
Index: include/apr_file_io.h
===
RCS file: /home/cvs/apr/include/apr_file_io.h,v
retrieving revision 1.141
diff -u -u -p -r1.141 apr_file_io.h
--- include/apr_file_io.h 15 Jun 2003 00:05:26 - 1.141
+++ include/apr_file_io.h 4 Jul 2003 01:55:23 -
@@ -655,6 +658,18 @@ APR_DECLARE(apr_status_t) apr_file_attrs
apr_fileattrs_t attributes,
apr_fileattrs_t attr_mask,
apr_pool_t *cont);
+
+/**
+ * Set the mtime of the specified file.
+ * @param fname The full path to the file (using / on all systems)
+ * @param mtime The mtime to apply to the file.
+ * @param pool The pool to use.
+ * @warning Platforms which do not implement this feature will return
+ * APR_ENOTIMPL.
+ */
+APR_DECLARE(apr_status_t) apr_file_mtime_set(const char *fname,
+ apr_time_t mtime,
+ apr_pool_t *pool);
/**
* Create a new directory on the file system.
Index: include/arch/win32/apr_arch_file_io.h
===
RCS file: /home/cvs/apr/include/arch/win32/apr_arch_file_io.h,v
retrieving revision 1.2
diff -u -u -p -r1.2 apr_arch_file_io.h
--- include/arch/win32/apr_arch_file_io.h 24 Jan 2003 17:15:56 -
1.2
+++ include/arch/win32/apr_arch_file_io.h 4 Jul 2003 01:55:24 -
@@ -133,10 +133,11 @@ void *res_name_from_filename(const char
#endif
/* Internal Flags for apr_file_open */
-#define APR_OPENINFO 0x4000/* Open without READ or WRITE access */
-#define APR_OPENLINK 0x2000/* Open a link itself, if supported */
-#define APR_READCONTROL 0x1000/* Read the file's owner/perms */
-#define APR_WRITECONTROL 0x0800/* Modifythe file's owner/perms */
+#define APR_OPENINFO 0x0010 /* Open without READ or WRITE access */
+#define APR_OPENLINK 0x0020 /* Open a link itself, if supported */
+#define APR_READCONTROL 0x0040 /* Read the file's owner/perms */
+#define APR_WRITECONTROL 0x0080 /* Modifythe file's owner/perms */
+#define APR_WRITEATTRS 0x0100 /* Modify the file's attributes */
/* Entries missing from the MSVC 5.0 Win32 SDK:
*/
Index: file_io/win32/open.c
===
RCS file: /home/cvs/apr/file_io/win32/open.c,v
retrieving revision 1.117
diff -u -u -p -r1.117 open.c
--- file_io/win32/open.c7 Jan 2003 00:52:54 - 1.117
+++ file_io/win32/open.c4 Jul 2003 01:55:23 -
@@ -309,7 +309,10 @@ APR_DECLARE(apr_status_t) apr_file_open(
if (flag & APR_WRITE) {
oflags |= GENERIC_WRITE;
}
-
+if (flag & APR_WRITEATTRS) {
+oflags |= FILE_WRITE_ATTRIBUTES;
+}
+
if (apr_os_level >= APR_WIN_NT)
sharemode |= FILE_SHARE_DELETE;
Index: file_io/win32/filestat.c
===
RCS file: /home/cvs/apr/file_io/win32/filestat.c,v
retrieving revision 1.80
diff -u -u -p -r1.80 filestat.c
--- file_io/win32/filestat.c24 May 2003 10:30:39 - 1.80
+++ file_io/win32/filestat.c4 Jul 2003 01:55:20 -
@@ -762,3 +762,37 @@ APR_DECLARE(apr_status_t) apr_file_attrs
return APR_SUCCESS;
}
+
+
+APR_DECLARE(apr_status_t) apr_file_mtime_set(const char *fname,
+ apr_time_t mtime,
+ apr_pool_t *pool)
+{
+apr_file_t *thefile;
+apr_status_t rv;
+
+rv = apr_file_open(&thefile, fname,
+ APR_READ | APR_WRITEATTRS,
+ APR_OS_DEFAULT, pool);
+if (!rv)
+{
+FILETIME file_ctime;
+FILETIME file_atime;
+FILETIME file_mtime;
+
+if (!GetFileTime(thefile->filehand,
+ &file_ctime, &file_atime, &file_mtime))
+rv = apr_get_os_error();
+else
+{
+AprTimeToFileTime(&file_mtime, mtime);
+if (!SetFileTime(thefile->filehand,
+ &file_ctime, &file_atime, &file_mtime))
+rv = apr_get_os_error();
+}
+
+apr_file_close(thefile);
+}
+
+return rv;
+}
Index: test/testfileinfo.c
===
RCS file: /home/cvs/apr/test/testfileinfo.c,v
retrieving revision 1.10
diff -u -u -p -r1.10 testfileinfo.c
--- test/testfileinfo.c 7 Feb 2003 12:33:00 - 1.10
+++ test/testfileinfo.c 4 Jul 2003 01:55:24 -
3@@ -59,6 +59,7 @@
#include "apr_general.h"
#include "apr_poll.h"
#include "apr_lib.h"
+#include "apr_time.h"
#include "test_apr.h"
#d
Re: setting timestamps
Cliff Woolley wrote: >On Wed, 2 Jul 2003, Ben Collins-Sussman wrote: > > > >>Subversion is already using apr_stat() to read the mtime of a file. >>But now we'd like to be able to *write* timestamps as well. >>Has this conversation been had before? >> >> > >I did some digging on marc.theaimsgroup and found this: > >http://marc.theaimsgroup.com/?l=apr-dev&m=104970018703214&w=2 > >It looks like a patch was submitted to do exactly that by Matt Kraai, >though it only provided an implementation for unix, and it seems to have >fallen through the cracks anyway. > This interface looks sane, and I can cobble up a Windows implementation in two eyeblinks. As for the patch itself: utimes() is a BSDism and may not be available on all systems. The Unix implementation needs to check for this, and use utime() instead if utimes() isn't available. -- Brane Äibej <[EMAIL PROTECTED]> http://www.xbc.nu/brane/
Re: setting timestamps
On Wed, 2 Jul 2003, Ben Collins-Sussman wrote:
> Subversion is already using apr_stat() to read the mtime of a file.
> But now we'd like to be able to *write* timestamps as well.
> Has this conversation been had before?
I did some digging on marc.theaimsgroup and found this:
http://marc.theaimsgroup.com/?l=apr-dev&m=104970018703214&w=2
It looks like a patch was submitted to do exactly that by Matt Kraai,
though it only provided an implementation for unix, and it seems to have
fallen through the cracks anyway. Justin reviewed the original patch,
though the revised one seems to have gotten no response. It was:
Index: file_io/unix/filestat.c
===
RCS file: /home/cvspublic/apr/file_io/unix/filestat.c,v
retrieving revision 1.65
diff -u -r1.65 filestat.c
--- file_io/unix/filestat.c 6 Mar 2003 09:21:24 - 1.65
+++ file_io/unix/filestat.c 4 Apr 2003 03:58:25 -
@@ -208,6 +208,30 @@
return apr_file_perms_set(fname, finfo.protection);
}
+APR_DECLARE(apr_status_t) apr_file_mtime_set(const char *fname,
+ apr_time_t mtime,
+ apr_pool_t *pool)
+{
+apr_status_t status;
+apr_finfo_t finfo;
+struct timeval tvp[2];
+
+status = apr_stat(&finfo, fname, APR_FINFO_ATIME, pool);
+if (!APR_STATUS_IS_SUCCESS(status)) {
+return status;
+}
+
+tvp[0].tv_sec = apr_time_sec(finfo.atime);
+tvp[0].tv_usec = apr_time_usec(finfo.atime);
+tvp[1].tv_sec = apr_time_sec(mtime);
+tvp[1].tv_usec = apr_time_usec(mtime);
+
+if (utimes(fname, tvp) == -1) {
+return errno;
+}
+return APR_SUCCESS;
+}
+
APR_DECLARE(apr_status_t) apr_stat(apr_finfo_t *finfo,
const char *fname,
apr_int32_t wanted, apr_pool_t *pool)
Index: include/apr_file_io.h
===
RCS file: /home/cvspublic/apr/include/apr_file_io.h,v
retrieving revision 1.138
diff -u -r1.138 apr_file_io.h
--- include/apr_file_io.h 3 Apr 2003 23:20:05 - 1.138
+++ include/apr_file_io.h 4 Apr 2003 03:58:27 -
@@ -658,6 +658,18 @@
apr_pool_t *cont);
/**
+ * Set the mtime of the specified file.
+ * @param fname The full path to the file (using / on all systems)
+ * @param mtime The mtime to apply to the file.
+ * @param pool The pool to use.
+ * @warning Platforms which do not implement this feature will return
+ * APR_ENOTIMPL.
+ */
+APR_DECLARE(apr_status_t) apr_file_mtime_set(const char *fname,
+ apr_time_t mtime,
+ apr_pool_t *pool);
+
+/**
* Create a new directory on the file system.
* @param path the path for the directory to be created. (use / on all
systems)
* @param perm Permissions for the new direcoty.
