Skip to content
This repository has been archived by the owner on Feb 16, 2025. It is now read-only.

cleanup keyNew #3152

Closed
markus2330 opened this issue Nov 3, 2019 · 9 comments · Fixed by #4193
Closed

cleanup keyNew #3152

markus2330 opened this issue Nov 3, 2019 · 9 comments · Fixed by #4193
Assignees
Labels
7p seven points cleanup cm2022s for university course lang/c triage needed Issue needs clarifications.
Milestone

Comments

@markus2330
Copy link
Contributor

markus2330 commented Nov 3, 2019

remove deprecated meta flags from keyNew (303-334), the documentation of it, and all tests that test them

/* deprecated flags */
case KEY_NAME:
name = va_arg (va, char *);
break;
case KEY_OWNER:
owner = va_arg (va, char *);
break;
case KEY_COMMENT:
keySetMeta (key, "comment", va_arg (va, char *));
break;
case KEY_UID:
elektraSetMetaInt (key, "uid", va_arg (va, int));
break;
case KEY_GID:
elektraSetMetaInt (key, "gid", va_arg (va, int));
break;
case KEY_DIR:
mode |= KDB_DIR_MODE;
break;
case KEY_MODE:
hasMode = 1;
mode |= va_arg (va, int);
break;
case KEY_ATIME:
elektraSetMetaInt (key, "atime", va_arg (va, time_t));
break;
case KEY_MTIME:
elektraSetMetaInt (key, "mtime", va_arg (va, time_t));
break;
case KEY_CTIME:
elektraSetMetaInt (key, "ctime", va_arg (va, time_t));
break;

@d3nwp
Copy link
Contributor

d3nwp commented Nov 21, 2019

I found some statements where KEY_META is used instead of some of the deprecated flags (like instead of using KEY_MODE its possible to use KEY_META with 2 parameters "mode" and the value as a string. I am trying to rewrite the testcases which uses the deprecated flags but the the test cases with KEY_DIR fails.

How can rewrite this one?
key = keyNew ("user", KEY_DIR, KEY_END);
succeed_if (keyGetMode (key) == 0700, "new key with KEY_DIR not 0700 by default");

I tried to use KEY_META, "mode", "0775" but it doesn't work :D

.. or shall I really remove all of the tests which use one of the deprecated flags.

@markus2330
Copy link
Contributor Author

I found some statements where KEY_META is used instead of some of the deprecated flags (like instead of using KEY_MODE its possible to use KEY_META with 2 parameters "mode" and the value as a string.

Yes, it is possible but mode currently does not have any meaning: if you look into doc/METADATA.md you see it is declared as deprecated, so you can remove also the occurrences in doc/METADATA.md so that we can get completely rid of this.

I am trying to rewrite the testcases which uses the deprecated flags but the the test cases with KEY_DIR fails.

You do not need to rewrite all testcases but only these which add a value to the test suite.

How can rewrite this one?
key = keyNew ("user", KEY_DIR, KEY_END);
succeed_if (keyGetMode (key) == 0700, "new key with KEY_DIR not 0700 by default");

This one does not test anything of relevance: It only tested the interplay of KEY_DIR and the resulting mode. As both things will be removed, the test does not test anything anymore.

I tried to use KEY_META, "mode", "0775" but it doesn't work :D

Yes 😄

.. or shall I really remove all of the tests which use one of the deprecated flags.

Maybe some of them test something in the core which is not tested by other tests. You can have a look at https://doc.libelektra.org/coverage/

But most likely all these tests can be remove without changing anything in the coverage of the core.

@d3nwp
Copy link
Contributor

d3nwp commented Nov 22, 2019

I am a little bit confused.. facepalm. I am working on 7fb0dee but where should I commit my changes....cuz I cant see the changes from 7fb... on the master branch. Do I need to commit my changes to that specific commit sha (and how xD...this is like commit-ception) ?!

@markus2330
Copy link
Contributor Author

Every commit gets a new SHA-1 and a pull request can have several commits. So you simply commit on top of your branch, not on top of a commit (this can happen if you make a checkout of a commit).

Simply git checkout to your branch to continue work. If everything else fails, make a new fork, make a new clone of that fork and copy all your files there.

@markus2330
Copy link
Contributor Author

Btw. calling git status often helps. If it does not help for you, post it here and we will translate it for you.

d3nwp added a commit to d3nwp/libelektra that referenced this issue Nov 24, 2019
@kodebach
Copy link
Member

kodebach commented Mar 3, 2021

The code now lives in another file, but some deprecated flags still exist:

/* deprecated flags */
case KEY_NAME:
name = va_arg (va, char *);
break;
case KEY_COMMENT:
keySetMeta (key, "comment", va_arg (va, char *));
break;

@kodebach kodebach added the cm2022s for university course label Mar 3, 2021
@markus2330 markus2330 added 7p seven points and removed good first issue labels Mar 4, 2021
@aaronabebe
Copy link
Contributor

So the problematic code is mostly in key.c now? And then ther docs and tests that use the flags need to be cleaned up as well right?

@kodebach
Copy link
Member

There may also be other parts of the codebase that still use the deprecated flags. In general, the issue is basically about removing the lines from keyVNew() and then updating the rest of the repository so that it compiles again and all tests work.

For docs and tests mostly means removing broken stuff, for other code you will need to replace KEY_COMMENT , "foo" with KEY_META, "comment", "foo". I don't think KEY_NAME is used anywhere, but if so, it's just a matter of changing the first argument to the keyNew () call to the string after KEY_NAME.

Also, if all unused KEY_* flags should be removed from kdb.h.in and kdbenum.c.

@markus2330 markus2330 added the triage needed Issue needs clarifications. label Oct 13, 2021
@flo91 flo91 self-assigned this Oct 20, 2021
@flo91
Copy link
Collaborator

flo91 commented Oct 20, 2021

[FLOSS H1]
I checked the codebase for occurrences of the two deprecated flags and got the following results.

For KEY_NAME:

File line number(s)
/src/libs/elektra/kdbenum.c 88
/src/libs/elektra/key.c 218
/src/bindings/glib/gelektra-key.h 25
/src/include/kdb.h.in 48
/tests/ctest/test_key.c 241
/src/plugins/specload/specload.c 580
/src/libs/elektra/keytest.c 414 (comment), 450 (comment), 497, 499, 501

For KEY_COMMENT:

File line number(s)
/src/libs/elektra/kdbenum.c 91
/src/libs/elektra/key.c 126 (comment), 221
/src/plugins/fstab/testmod_fstab.c 63-79
/src/plugins/iconv/data_latin1.c 14, 19, 23, 27, 31
/src/plugins/iconv/data_utf8.c 14, 19, 23, 27, 31
/src/plugins/validation/testmod_validation.c 36-39, 62-65
/tests/data/data_key.c 14, 18, 22, 26, 30, 34, 38, 42, 46
/tests/data/data_keyset.c 15-113
/tests/data/data_ns.c 15-512
/tests/data/data_others.c 17-337

@flo91 flo91 added the lang/c label Oct 20, 2021
flo91 added a commit to flo91/libelektra that referenced this issue Nov 3, 2021
…and KEY_VALUE are still used or mentioned in code comments. (see ElektraInitiative#3152)
flo91 added a commit to flo91/libelektra that referenced this issue Nov 25, 2021
and all references to them, replace with other methods
(e.g. use KEY_META for comments)
closes issue ElektraInitiative#3152 (cleanup keyNew)
flo91 added a commit to flo91/libelektra that referenced this issue Nov 25, 2021
and all references to them, replace with other methods
(e.g. use KEY_META for comments)
closes issue ElektraInitiative#3152 (cleanup keyNew)
flo91 added a commit to flo91/libelektra that referenced this issue Nov 25, 2021
and all references to them, replace with other methods
(e.g. use KEY_META for comments)
closes issue ElektraInitiative#3152 (cleanup keyNew)
flo91 added a commit to flo91/libelektra that referenced this issue Nov 25, 2021
and all references to them, replace with other methods
(e.g. use KEY_META for comments)
closes issue ElektraInitiative#3152 (cleanup keyNew)
flo91 added a commit to flo91/libelektra that referenced this issue Nov 29, 2021
and all references to them, replace with other methods
(e.g. use KEY_META for comments)
closes issue ElektraInitiative#3152 (cleanup keyNew)
flo91 added a commit to flo91/libelektra that referenced this issue Dec 16, 2021
and all references to them, replace with other methods
(e.g. use KEY_META for comments)
closes issue ElektraInitiative#3152 (cleanup keyNew)
flo91 added a commit to flo91/libelektra that referenced this issue Dec 17, 2021
and all references to them, replace with other methods
(e.g. use KEY_META for comments)
closes issue ElektraInitiative#3152 (cleanup keyNew)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
7p seven points cleanup cm2022s for university course lang/c triage needed Issue needs clarifications.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants