do not create .cfg file for config protected files which are not modified by the user. #21
do not create .cfg file for config protected files which are not modified by the user. #21mrmonopoly-cyber wants to merge 25 commits intogentoo:masterfrom
Conversation
|
Thanks, more interest in portage-utils is great. Could you squash the commits here please into one, with a commit message describing what you said (briefly) in the PR? |
|
general remark, could you try and add some descriptive content to the commit messages? Something like: libq/foo: add variant of Y with length avoid having to copy strings that are not zero-terminated to use Y by providing a version that takes a length |
qmerge.c
Outdated
| #include "xsystem.h" | ||
|
|
||
|
|
||
| //patch |
libq/cur_sys_pkg.c
Outdated
| //functions | ||
| static unsigned int conv_char_int(char dig) | ||
| { | ||
| if((int) dig > 57) |
There was a problem hiding this comment.
what is 57 here? perhaps use the ASCII representations? e.g. if (dig > '9')
libq/cur_sys_pkg.c
Outdated
| int temp1,temp2; | ||
| for(int i=0;i<HASH_SIZE;++i) | ||
| { | ||
| temp1=conv_char_int(hash1[i]); |
There was a problem hiding this comment.
feels like you really don't want the hex-encoded hashes here, but the binary ones? Perhaps you can just grab those instead?
libq/cur_sys_pkg.c
Outdated
| return (int )dig - 48; | ||
| } | ||
|
|
||
| static int compare_hash_num(char *hash1,char*hash2) |
There was a problem hiding this comment.
somehow it feels like you really could implement this using return strncmp(hash1, hash2, HASH_SIZE)?
There was a problem hiding this comment.
yeah, It's the same thing, also easier. I will change it. Thx
libq/cur_sys_pkg.c
Outdated
|
|
||
| strcat(package_name,datom->CATEGORY); | ||
| strcat(package_name,"/"); | ||
| strcat(package_name,datom->PN); |
There was a problem hiding this comment.
Hmmm, atom_format("%{CAT}/%{PN}", datom) isn't sufficient here?
There was a problem hiding this comment.
i didn't see that, thx. I will check
libq/cur_sys_pkg.c
Outdated
|
|
||
| return out; | ||
| } | ||
| MD5_Init(&ctx); |
There was a problem hiding this comment.
really, there's a libq/hash that provides a several functions for you to reuse instead of re-implementing this
There was a problem hiding this comment.
Yeah, i see that recently. Recently i was checking to see if i could improve the patch and i notice that. I already changed it in my repo
libq/cur_sys_pkg.c
Outdated
|
|
||
| MD5_Init(&ctx); | ||
| MD5_Update(&ctx,str,len); | ||
| MD5_Final(hex_buf,&ctx); |
There was a problem hiding this comment.
it seems like a better idea to extend hash.h to add a hash_buffer or something, that would also make a fine separate commit that would be easily reviewed and merged
libq/cur_sys_pkg.c
Outdated
|
|
||
| static void read_file_add_data(cur_pkg_tree_node **root,char *package_name) | ||
| { | ||
| FILE *CONTENTS=fopen("./CONTENTS","r"); |
There was a problem hiding this comment.
libq/tree has means to navigate the VDB and retrieve contents. I don't think you should be doing this manually. In particular libq/contents is there to parse a line from a contents file, you should use that
libq/cur_sys_pkg.c
Outdated
| xchdir(path); | ||
| dir=opendir("."); | ||
|
|
||
| while(!find_it && (dirent_struct=readdir(dir)) != NULL) |
There was a problem hiding this comment.
this feels a lot like libq/tree's business
|
It seems you already did some rewrites and stuff. Can you squash your changes and split them out in functional parts which can be reviewed individually? |
|
I see, thanks for the suggestion. I will update the code based on the suggestion. |
…sh with MD5 from a string given by the user. The length of the string is optional, if the string given is -1 the function will compute the length by itself
…t the function accept also the length of the string, is the length given is <= 0 the function will compute the length by itself. Also now the function can also parse line where the path of the file has blank spaces.
…le.am to include libq/cur_sys_pkg.c and libq/cur_sys_pkg.h and regenerating Makefiles.in
…e creation of a tree with the function create_cur_pkg_tree. With this function the tree is saved in the root pointer and it consists of a tree with all the obj files in the CONTENS file of the package. Then with the function is_default is possible to check if a certain file in the filesystem, if it belongs to the package of the tree, has the same hash of the entry in the tree. If this is the case the return True else False. At the end there is also a function to free the tree.
…ature of overwrite the config protected files if they have the same hash of the entry in /var/db/pkg/[package_name]/CONTENTS
mrmonopoly-cyber
force-pushed
the
default-file
branch
from
567e4c0 to
b17d767
Compare
|
i remove the category check in the tree. I do this because i fixed the research in the /var/db/pkg to load in the tree only the CONTENTS file the package. I navigate manually because, even after searching in the libq/tree.h structs i was not able to find a path o a fd of the CONTENTS of the package in /var/db/pkg. If there is a way i will adapt the code to use to make it consistent. |
|
Also i notice that the libraries used in the file libq/hash.c are deprecated. In the function i added (hash_from_string) i used the same libraries for consitency, but i can change it to work with the newest, at least my function. If that's ok i will change it |
|
There was some bugs that i fixed. I did it only now because i had no data to test the qmerge. Now everything is fixed and working. |
updated hash_from_string to choose the type of hash to compute. If the hash type is not supported return NULL hash_from_string no longed allocate buffer on Heap, return a pointer to the static updated contract of hash_from_string updated the use or hash_from_string in the repo
|
I resolve all the problem indicated above. with my last commit Tell me if there is any problem |
retrieve the data, it uses the function in libq/tree.h to get CONTENTS of the package, then it parses in the same way as the pkg_unmerge function in qmerge.c does. Still to refactor but it works.
bug which depend on something related to libq/tree.h. In particularly if you use qmerge with a list of packages in input where one package, or a dependency, is installed at least twice the function tree_pkg_meta_get of libq/tree.h will return, after the first time, a string with all the line in the file CONTENTS but the '\n' char used as separator will be replaced with '\0'. For that reason may happen that the tree generated in cur_pkg_tree is NULL. The program will not crush for this reason but the comparison made to determine if a .cfg file should be created is wrong and create a file every time. Other than that the patch now is ready and work properly with libq/tree.h without manually searching or opening files.
|
I fix the old problem of navigating the filesystem to search the CONTENTS file. I also reduce the size of the file libq/cur_sys_pkc.c. Also fixed a little include bug in libq/tree.h about a constant ( I put it in a different commit so it's easier to remove if that create a problem, it should not but i don't know all the code base). I encounter in a bug which i cannot fix. I described it in detail in the last commit. But to reproduce it i use qmerge to install nano and ncurses using this sintax: |
…this function modifies the buffer obtained from tree_pkg_meta_get(pkg_ctx, CONTENTS) and this lead to problems in the future parsing of that string. adding a strdup fix the problem, not very elegant but works and solves the problem
…, still reamin in qmerge.c
to reset the string modified by the parser function. Fix the bug in qmerge.c where the parsing of the file CONTENTS of a package in pkg_unmerge also modified the buffer in libq/tree.h. Now during the pkg_unmerge and during the creation of the tree in cur_sys_pkg.c the buffer is parsed and then restored. Also removed the dynamic memory allocation in qmerge.c and cur_sys_pkc.c.
|
I fix all the bugs and problem, also restoring the old contents.c implementation. Everything now works as intended. cur_sys_pkg.c no longer manually search the file and for parsing uses the original function in contents.c. In the commits i explain also the bug i fixed in the parsing of the CONTENTS data in pkg_unmerge in qmerge.c. Tell me if there is something else to fix or change in the patch. For now i don't know what to other than wait you review. |
Alternative to the implementation in PR #21, so as to reuse the same hashing code. We could add the interface to compute multiple hashes from the same string when that's actually necessary. Signed-off-by: Fabian Groffen <grobian@gentoo.org>
This seems necessary for PR #21, but keep the original code structure largely in-tact. Signed-off-by: Fabian Groffen <grobian@gentoo.org>
…string, insert missing signature in the header contents.h
…o about the package is not reliable and can be NULL. Using mpkg fix the problem.
3 participants
The Patch enable the qmerge to understand if a config protected file has the same hash of the entry in /var/db/pkg/[package_name]/CONTENTS. In this way if this is the case the qmerge overwrite without generating a .cfg file. The reason for that is pretty seample, the amount of .cfg file generated by qmerge is enormous in comparison with the emerge. This patch solves the problem. If a file in config protected is has a different hash from the entry in /var/db/pkg the .cfg will be generated. To do so my patch generate a tree with all the obj files of the packages the qmerge is currently installing. This process is made in the pkg_fetch funtion to also take the dependencies. Then in merge_tree_at function it checks if the hash of the file in the filesystem is in the tree, if it's not the .cfg will be generate otherwise this file will be managed as if it is not a config protected file at all. I also generalize the function in lib/contents.c in such a way that you can pass the length of the line. I still put a macro to preserve the compatibility with the old declaration. In the same function i also modify the implementation. In this way the function will also manage obj files which has space inside their name. I tried, where possible, to adapt my code to the standard of the project without reinventing anything if already present. The patch does not add a notable performance overhead and i try to make it as efficient as possible. I try to use the internal test of the repo but i could not understand what were they really test. All i can say is that, in the company where i work, we are using in production for the roll of the servers and everything is ok with the patch.