diff mbox

[pacman-dev,v2] libalpm: parse {check, make}depends when reading database

Message ID 20181009010515.13151-1-morganamilo@gmail.com
State Accepted, archived
Delegated to: Andrew Gregory
Headers show

Commit Message

morganamilo Oct. 9, 2018, 1:05 a.m. UTC
Commit 0994893b0e6b627d45a63884ac01af7d0967eff2 added the
alpm_pkg_get_{make,check}depends functions but forgot to include
logic for parsing these fields from the database. As a result these
functions will always return an empty list.

This commit adds the parsing logic.

Signed-off-by: morganamilo <morganamilo@gmail.com>
---

Only after making this patch do I see FS#60347. V2 addes the extra lazy
loading code pointed out in the comments.

As a result this patch is basically identical to Alexandre Garnier's patch.
But as they have not submitted their patch to the mailing list I'll submit mine.

Comments

Allan McRae Oct. 9, 2018, 1:15 a.m. UTC | #1
On 9/10/18 11:05 am, morganamilo wrote:
>  static struct pkg_operations local_pkg_ops = {
> -	.get_base        = _cache_get_base,
> -	.get_desc        = _cache_get_desc,
> -	.get_url         = _cache_get_url,
> -	.get_builddate   = _cache_get_builddate,
> -	.get_installdate = _cache_get_installdate,
> -	.get_packager    = _cache_get_packager,
<snip>
> +	.get_base         = _cache_get_base,
> +	.get_desc         = _cache_get_desc,
> +	.get_url          = _cache_get_url,
> +	.get_builddate    = _cache_get_builddate,
> +	.get_installdate  = _cache_get_installdate,
...

Is there any objection to killing this formatting rather than having a
lot of uselessness in the patch.

A
Eli Schwartz Oct. 9, 2018, 1:20 a.m. UTC | #2
On 10/8/18 9:15 PM, Allan McRae wrote:
> On 9/10/18 11:05 am, morganamilo wrote:
> <snip>
>> +	.get_base         = _cache_get_base,
>> +	.get_desc         = _cache_get_desc,
>> +	.get_url          = _cache_get_url,
>> +	.get_builddate    = _cache_get_builddate,
>> +	.get_installdate  = _cache_get_installdate,
> ...
> 
> Is there any objection to killing this formatting rather than having a
> lot of uselessness in the patch.

I'm rather meh about keeping things aligned like this in the first
place, so I'm okay with killing the formatting.

I mean, indenting the beginning of the line makes sense. Making variable
assignments line up is useless IMO. But if you will do it then you
should leave space for it to grow...
morganamilo Oct. 9, 2018, 1:20 a.m. UTC | #3
On 09/10/2018 2:15 am, Allan McRae wrote:
> On 9/10/18 11:05 am, morganamilo wrote:
>>  static struct pkg_operations local_pkg_ops = {
>> -	.get_base        = _cache_get_base,
>> -	.get_desc        = _cache_get_desc,
>> -	.get_url         = _cache_get_url,
>> -	.get_builddate   = _cache_get_builddate,
>> -	.get_installdate = _cache_get_installdate,
>> -	.get_packager    = _cache_get_packager,
> <snip>
>> +	.get_base         = _cache_get_base,
>> +	.get_desc         = _cache_get_desc,
>> +	.get_url          = _cache_get_url,
>> +	.get_builddate    = _cache_get_builddate,
>> +	.get_installdate  = _cache_get_installdate,
> ...
> 
> Is there any objection to killing this formatting rather than having a
> lot of uselessness in the patch.
> 
> A
> 

I could split adding the fields and the formatting to two patches if you
prefer? Easier to read diffs and the struct stays pretty.
Andrew Gregory Oct. 9, 2018, 2:19 a.m. UTC | #4
On 10/09/18 at 02:20am, Morgan Adamiec wrote:
> On 09/10/2018 2:15 am, Allan McRae wrote:
> > On 9/10/18 11:05 am, morganamilo wrote:
> >>  static struct pkg_operations local_pkg_ops = {
> >> -	.get_base        = _cache_get_base,
> >> -	.get_desc        = _cache_get_desc,
> >> -	.get_url         = _cache_get_url,
> >> -	.get_builddate   = _cache_get_builddate,
> >> -	.get_installdate = _cache_get_installdate,
> >> -	.get_packager    = _cache_get_packager,
> > <snip>
> >> +	.get_base         = _cache_get_base,
> >> +	.get_desc         = _cache_get_desc,
> >> +	.get_url          = _cache_get_url,
> >> +	.get_builddate    = _cache_get_builddate,
> >> +	.get_installdate  = _cache_get_installdate,
> > ...
> > 
> > Is there any objection to killing this formatting rather than having a
> > lot of uselessness in the patch.
> > 
> > A
> > 
> 
> I could split adding the fields and the formatting to two patches if you
> prefer? Easier to read diffs and the struct stays pretty.

Kill the formatting.  We've had this discussion before, for this very
feature even:
https://lists.archlinux.org/pipermail/pacman-dev/2017-January/021784.html
Allan McRae Oct. 9, 2018, 3:21 a.m. UTC | #5
On 9/10/18 12:19 pm, Andrew Gregory wrote:
> On 10/09/18 at 02:20am, Morgan Adamiec wrote:
>> On 09/10/2018 2:15 am, Allan McRae wrote:
>>> On 9/10/18 11:05 am, morganamilo wrote:
>>>>  static struct pkg_operations local_pkg_ops = {
>>>> -	.get_base        = _cache_get_base,
>>>> -	.get_desc        = _cache_get_desc,
>>>> -	.get_url         = _cache_get_url,
>>>> -	.get_builddate   = _cache_get_builddate,
>>>> -	.get_installdate = _cache_get_installdate,
>>>> -	.get_packager    = _cache_get_packager,
>>> <snip>
>>>> +	.get_base         = _cache_get_base,
>>>> +	.get_desc         = _cache_get_desc,
>>>> +	.get_url          = _cache_get_url,
>>>> +	.get_builddate    = _cache_get_builddate,
>>>> +	.get_installdate  = _cache_get_installdate,
>>> ...
>>>
>>> Is there any objection to killing this formatting rather than having a
>>> lot of uselessness in the patch.
>>>
>>> A
>>>
>>
>> I could split adding the fields and the formatting to two patches if you
>> prefer? Easier to read diffs and the struct stays pretty.
> 
> Kill the formatting.  We've had this discussion before, for this very
> feature even:
> https://lists.archlinux.org/pipermail/pacman-dev/2017-January/021784.html
> .
> 

OK - a patch to kill the formating, then this patch rebased on top.

A
diff mbox

Patch

diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c
index 7c2f96b8..12b13a12 100644
--- a/lib/libalpm/be_local.c
+++ b/lib/libalpm/be_local.c
@@ -153,6 +153,18 @@  static alpm_list_t *_cache_get_optdepends(alpm_pkg_t *pkg)
 	return pkg->optdepends;
 }
 
+static alpm_list_t *_cache_get_makedepends(alpm_pkg_t *pkg)
+{
+	LAZY_LOAD(INFRQ_DESC);
+	return pkg->makedepends;
+}
+
+static alpm_list_t *_cache_get_checkdepends(alpm_pkg_t *pkg)
+{
+	LAZY_LOAD(INFRQ_DESC);
+	return pkg->checkdepends;
+}
+
 static alpm_list_t *_cache_get_conflicts(alpm_pkg_t *pkg)
 {
 	LAZY_LOAD(INFRQ_DESC);
@@ -303,36 +315,38 @@  static int _cache_force_load(alpm_pkg_t *pkg)
  * logic.
  */
 static struct pkg_operations local_pkg_ops = {
-	.get_base        = _cache_get_base,
-	.get_desc        = _cache_get_desc,
-	.get_url         = _cache_get_url,
-	.get_builddate   = _cache_get_builddate,
-	.get_installdate = _cache_get_installdate,
-	.get_packager    = _cache_get_packager,
-	.get_arch        = _cache_get_arch,
-	.get_isize       = _cache_get_isize,
-	.get_reason      = _cache_get_reason,
-	.get_validation  = _cache_get_validation,
-	.has_scriptlet   = _cache_has_scriptlet,
-	.get_licenses    = _cache_get_licenses,
-	.get_groups      = _cache_get_groups,
-	.get_depends     = _cache_get_depends,
-	.get_optdepends  = _cache_get_optdepends,
-	.get_conflicts   = _cache_get_conflicts,
-	.get_provides    = _cache_get_provides,
-	.get_replaces    = _cache_get_replaces,
-	.get_files       = _cache_get_files,
-	.get_backup      = _cache_get_backup,
-
-	.changelog_open  = _cache_changelog_open,
-	.changelog_read  = _cache_changelog_read,
-	.changelog_close = _cache_changelog_close,
-
-	.mtree_open      = _cache_mtree_open,
-	.mtree_next      = _cache_mtree_next,
-	.mtree_close     = _cache_mtree_close,
-
-	.force_load      = _cache_force_load,
+	.get_base         = _cache_get_base,
+	.get_desc         = _cache_get_desc,
+	.get_url          = _cache_get_url,
+	.get_builddate    = _cache_get_builddate,
+	.get_installdate  = _cache_get_installdate,
+	.get_packager     = _cache_get_packager,
+	.get_arch         = _cache_get_arch,
+	.get_isize        = _cache_get_isize,
+	.get_reason       = _cache_get_reason,
+	.get_validation   = _cache_get_validation,
+	.has_scriptlet    = _cache_has_scriptlet,
+	.get_licenses     = _cache_get_licenses,
+	.get_groups       = _cache_get_groups,
+	.get_depends      = _cache_get_depends,
+	.get_optdepends   = _cache_get_optdepends,
+	.get_makedepends  = _cache_get_makedepends,
+	.get_checkdepends = _cache_get_checkdepends,
+	.get_conflicts    = _cache_get_conflicts,
+	.get_provides     = _cache_get_provides,
+	.get_replaces     = _cache_get_replaces,
+	.get_files        = _cache_get_files,
+	.get_backup       = _cache_get_backup,
+
+	.changelog_open   = _cache_changelog_open,
+	.changelog_read   = _cache_changelog_read,
+	.changelog_close  = _cache_changelog_close,
+
+	.mtree_open       = _cache_mtree_open,
+	.mtree_next       = _cache_mtree_next,
+	.mtree_close      = _cache_mtree_close,
+
+	.force_load       = _cache_force_load,
 };
 
 static int checkdbdir(alpm_db_t *db)
@@ -773,6 +787,10 @@  static int local_db_read(alpm_pkg_t *info, int inforeq)
 				READ_AND_SPLITDEP(info->depends);
 			} else if(strcmp(line, "%OPTDEPENDS%") == 0) {
 				READ_AND_SPLITDEP(info->optdepends);
+			} else if(strcmp(line, "%MAKEDEPENDS%") == 0) {
+				READ_AND_SPLITDEP(info->makedepends);
+			} else if(strcmp(line, "%CHECKDEPENDS%") == 0) {
+				READ_AND_SPLITDEP(info->checkdepends);
 			} else if(strcmp(line, "%CONFLICTS%") == 0) {
 				READ_AND_SPLITDEP(info->conflicts);
 			} else if(strcmp(line, "%PROVIDES%") == 0) {
diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
index 5009a7da..af94b2d5 100644
--- a/lib/libalpm/be_sync.c
+++ b/lib/libalpm/be_sync.c
@@ -700,17 +700,9 @@  static int sync_db_read(alpm_db_t *db, struct archive *archive,
 			} else if(strcmp(line, "%OPTDEPENDS%") == 0) {
 				READ_AND_SPLITDEP(pkg->optdepends);
 			} else if(strcmp(line, "%MAKEDEPENDS%") == 0) {
-				/* currently unused */
-				while(1) {
-					READ_NEXT();
-					if(strlen(line) == 0) break;
-				}
+				READ_AND_SPLITDEP(pkg->makedepends);
 			} else if(strcmp(line, "%CHECKDEPENDS%") == 0) {
-				/* currently unused */
-				while(1) {
-					READ_NEXT();
-					if(strlen(line) == 0) break;
-				}
+				READ_AND_SPLITDEP(pkg->checkdepends);
 			} else if(strcmp(line, "%CONFLICTS%") == 0) {
 				READ_AND_SPLITDEP(pkg->conflicts);
 			} else if(strcmp(line, "%PROVIDES%") == 0) {