Skip to content

Allow packages to use ISO 8601 dates in their PackageInfo.g files (so YYYY-MM-DD instead of DD/MM/YYYY) #3355

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
olexandr-konovalov merged 1 commit into from
Feb 20, 2021
Merged

Allow packages to use ISO 8601 dates in their PackageInfo.g files (so YYYY-MM-DD instead of DD/MM/YYYY) #3355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 54 additions & 6 deletions lib/package.gi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ end );
#F AddPackageInfo( files )
##
BindGlobal( "AddPackageInfos", function( files, pkgdir, ignore )
local file, record, pkgname, version;
local file, record, pkgname, version, date, dd, mm;
Copy link
Member

@olexandr-konovalov olexandr-konovalov Apr 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, we call ValidatePackageInfo from LoadPackage but not from AddPackageInfos. I wonder if we should do that in AddPackageInfos too...

Copy link
Member

@fingolfin fingolfin Apr 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps, but that goes beyond the purpose of this PR, doesn't it?

for file in files do
# Read the `PackageInfo.g' file.
Unbind( GAPInfo.PackageInfoCurrent );
Expand Down Expand Up @@ -241,6 +241,30 @@ BindGlobal( "AddPackageInfos", function( files, pkgdir, ignore )
elif IsRecord( record.PackageDoc ) then
record.PackageDoc:= [ record.PackageDoc ];
fi;

# Normalize the format of 'Date', i.e. if it is the format yyyy-mm-dd
# then we change it to dd/mm/yyyy. When other tools have adapted to
# the yyyy-mm-dd format we can normalize to that format and at some
# point in the future get rid of this code.
if record.Date{[5,8]} = "--" then
date := List( SplitString( record.Date, "-" ), Int);
date := Permuted(date, (1,3)); # date = [dd,mm,yyyy]
# generate the day and month strings
# if the day has only one digit we have to add a 0
if date[1] < 10 then
dd := Concatenation("0", String(date[1]));
else
dd := String(date[1]);
fi;
# if the month has only one digit we have to add a 0
if date[2] < 10 then
mm := Concatenation("0", String(date[2]));
else
mm := String(date[2]);
fi;
record.Date := Concatenation(dd, "/", mm, "/", String(date[3]));
fi;

if IsHPCGAP then
# FIXME: we make the package info record immutable, to
# allow access from multiple threads; but that in turn
Expand Down Expand Up @@ -2130,7 +2154,7 @@ InstallGlobalFunction( DeclareAutoreadableVariables,
InstallGlobalFunction( ValidatePackageInfo, function( info )
local record, pkgdir, i, IsStringList, IsRecordList, IsProperBool, IsURL,
IsFilename, IsFilenameList, result, TestOption, TestMandat, subrec,
list;
list, CheckDateValidity;

if IsString( info ) then
if IsReadableFile( info ) then
Expand Down Expand Up @@ -2197,10 +2221,34 @@ InstallGlobalFunction( ValidatePackageInfo, function( info )
TestMandat( record, "Version",
x -> IsString(x) and 0 < Length(x) and x[1] <> '=',
"a nonempty string that does not start with `='" );
TestMandat( record, "Date",
x -> IsString(x) and Length(x) = 10 and x{ [3,6] } = "//"
and ForAll( x{ [1,2,4,5,7,8,9,10] }, IsDigitChar ),
"a string of the form `dd/mm/yyyy'" );

# check if the date is valid
CheckDateValidity := function(x)
local date;

if not ( IsString(x) and Length(x) = 10
and ( ( ForAll( x{ [1,2,4,5,7,8,9,10] }, IsDigitChar )
and x{ [3,6] } = "//" ) or ( ForAll( x{ [1,2,3,4,6,7,9,10] }, IsDigitChar ) and x{ [5,8] } = "--" ) ) ) then
return false;
elif x{ [3,6] } = "//" then # for the old format split at '/'
date := List( SplitString( x, "/" ), Int);
elif x{ [5,8] } = "--" then # for the yyyy-mm-dd format split at '-'
date := List( SplitString( x, "-" ), Int);
date := date{ [3,2,1] }; # sort such that date=[dd,mm,yyyy]
fi;
return date[2] in [1..12] and date[3] >= 1999 # GAP 4 appeared in 1999
and date[1] in [1..DaysInMonth( date[2], date[3] )];
end;

TestMandat( record, "Date", CheckDateValidity, Concatenation( "a string of the form yyyy-mm-dd or dd/mm/yyyy",
" that represents a date since 1999") );

# If the date is in the format `dd/mm/yyyy` a message is printed
# code to be used after an adaption period for packages
#if IsBound( record.Date ) and CheckDateValidity( record.Date ) and record.Date{ [3,6] } = "//" then
# Info( InfoPackageLoading, 2, Concatenation( record.PackageName, ": Please be advised to change the date format to `yyyy-mm-dd`") );
#fi;

TestOption( record, "License",
x -> IsString(x) and 0 < Length(x),
"a nonempty string containing an SPDX ID" );
Expand Down
117 changes: 116 additions & 1 deletion tst/testinstall/package.tst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,8 @@ gap> ValidatePackageInfo(rec());
#E component `Subtitle' must be bound to a string
#E component `Version' must be bound to a nonempty string that does not start\
with `='
#E component `Date' must be bound to a string of the form `dd/mm/yyyy'
#E component `Date' must be bound to a string of the form yyyy-mm-dd or dd/mm\
/yyyy that represents a date since 1999
#E component `ArchiveURL' must be bound to a string started with http://, htt\
ps:// or ftp://
#E component `ArchiveFormats' must be bound to a string
Expand All @@ -222,6 +223,120 @@ gap> info := rec(
> PackageName := "pkg",
> Subtitle := "desc",
> Version := "0",
> Date := "01/20/2015",
> ArchiveURL := "https://",
> ArchiveFormats := "",
> Status := "other",
> README_URL := "https://",
> PackageInfoURL := "https://",
> AbstractHTML := "",
> PackageWWWHome := "https://",
> PackageDoc := rec(),
> AvailabilityTest := ReturnTrue,
> );;
gap> ValidatePackageInfo(info);
#E component `Date' must be bound to a string of the form yyyy-mm-dd or dd/mm\
/yyyy that represents a date since 1999
#E component `BookName' must be bound to a string
#E component `ArchiveURLSubset' must be bound to a list of strings denoting r\
elative paths to readable files or directories
#E component `HTMLStart' must be bound to a string denoting a relative path t\
o a readable file
#E component `PDFFile' must be bound to a string denoting a relative path to \
a readable file
#E component `SixFile' must be bound to a string denoting a relative path to \
a readable file
#E component `LongTitle' must be bound to a string
false
gap> info := rec(
> PackageName := "pkg",
> Subtitle := "desc",
> Version := "0",
> Date := "2013-05-22",
> ArchiveURL := "https://",
> ArchiveFormats := "",
> Status := "other",
> README_URL := "https://",
> PackageInfoURL := "https://",
> AbstractHTML := "",
> PackageWWWHome := "https://",
> PackageDoc := rec(),
> AvailabilityTest := ReturnTrue,
> );;
gap> ValidatePackageInfo(info);
#E component `BookName' must be bound to a string
#E component `ArchiveURLSubset' must be bound to a list of strings denoting r\
elative paths to readable files or directories
#E component `HTMLStart' must be bound to a string denoting a relative path t\
o a readable file
#E component `PDFFile' must be bound to a string denoting a relative path to \
a readable file
#E component `SixFile' must be bound to a string denoting a relative path to \
a readable file
#E component `LongTitle' must be bound to a string
false
gap> info := rec(
> PackageName := "pkg",
> Subtitle := "desc",
> Version := "0",
> Date := "2013-22-05",
> ArchiveURL := "https://",
> ArchiveFormats := "",
> Status := "other",
> README_URL := "https://",
> PackageInfoURL := "https://",
> AbstractHTML := "",
> PackageWWWHome := "https://",
> PackageDoc := rec(),
> AvailabilityTest := ReturnTrue,
> );;
gap> ValidatePackageInfo(info);
#E component `Date' must be bound to a string of the form yyyy-mm-dd or dd/mm\
/yyyy that represents a date since 1999
#E component `BookName' must be bound to a string
#E component `ArchiveURLSubset' must be bound to a list of strings denoting r\
elative paths to readable files or directories
#E component `HTMLStart' must be bound to a string denoting a relative path t\
o a readable file
#E component `PDFFile' must be bound to a string denoting a relative path to \
a readable file
#E component `SixFile' must be bound to a string denoting a relative path to \
a readable file
#E component `LongTitle' must be bound to a string
false
gap> info := rec(
> PackageName := "pkg",
> Subtitle := "desc",
> Version := "0",
> Date := "2013-05-22-",
> ArchiveURL := "https://",
> ArchiveFormats := "",
> Status := "other",
> README_URL := "https://",
> PackageInfoURL := "https://",
> AbstractHTML := "",
> PackageWWWHome := "https://",
> PackageDoc := rec(),
> AvailabilityTest := ReturnTrue,
> );;
gap> ValidatePackageInfo(info);
#E component `Date' must be bound to a string of the form yyyy-mm-dd or dd/mm\
/yyyy that represents a date since 1999
#E component `BookName' must be bound to a string
#E component `ArchiveURLSubset' must be bound to a list of strings denoting r\
elative paths to readable files or directories
#E component `HTMLStart' must be bound to a string denoting a relative path t\
o a readable file
#E component `PDFFile' must be bound to a string denoting a relative path to \
a readable file
#E component `SixFile' must be bound to a string denoting a relative path to \
a readable file
#E component `LongTitle' must be bound to a string
false
gap> info := rec(
> PackageName := "pkg",
> Subtitle := "desc",
> Version := "0",
> Date := "01/02/3000",
> ArchiveURL := "https://",
> ArchiveFormats := "",
Expand Down