zqqw / pakku

Pacman wrapper with AUR support
GNU General Public License v3.0
39 stars 3 forks source link

Libalpm13 fixups #13

Closed Ckath closed 3 years ago

Ckath commented 3 years ago

some things were stopping it from compiling with the new version of libalpm, heres my attempt at fixing them. from what I can tell alpm_option_add_architecture matches the now removed alpm_option_set_arch, the signature is the same and it seems to work for what its worth at least.

the zsh completion patch had some issues and seemingly broken parts that broke it as well so I also fixed that up.

zqqw commented 3 years ago
New version:
/** Sets the allowed package architecture.
 * @param handle the context handle
 * @param arches the architecture to set
 */
int alpm_option_set_architectures(alpm_handle_t *handle, alpm_list_t *arches);

Old version:
/** Sets the allowed package architecture.
 * @param handle the context handle
 * @param arch the architecture to set
 */
int alpm_option_set_arch(alpm_handle_t *handle, const char *arch);

The parameters are different types, not 100% sure this patch works properly, even if it builds. The new type is a struct containing a void* data entry, defined in /usr/include/alpm_list.h but thank you, you may be right IDK yet!

Ckath commented 3 years ago

I'm not sure how it looked when it was alpm_option_set_arch but from the description of alpm_option_add_architecture being it Adds an allowed package architecture and then get/set only being used to get and set the entire list of those architectures I assumed alpm_option_add_architecture would more or less be the same as setting it to one allowed on in the old system. also the add_architecture signature is 100% 1:1 with types of the old set_arch:

int alpm_option_add_architecture (alpm_handle_t *handle, const char *arch)

for a moment I considered the replacement would be alpm_option_set_architectures but getting that to work the same as alpm_option_set_arch would look something like:

handle.addArch(arch)
list = handle.getArches()
handle.setArches(list)

which makes the whole get/set step look redundant over just the add.

I havent gotten any definite answer though on if my assumptions are correct and I havent worked with libalpm before so its all up for change at this point. is there any case I could test the arch being set correctly? I havent really run into it before, maybe try to find a package that doesnt support my arch and see if that'll error? would that implicated it works correctly?

further thoughts after bothering some folks in #archlinux, I think using add instead of set can be ignored anyways because its only called in the withAlpm function with a new handle. meaning add will only ever be called once per handle anyway. so any issues that could arrise from calling an add instead of a set arent at play here. my assumptions also seem correct so far that the add_architecture does indeed add it to the list of accepted architectures, effectively doing the same as the old set_arch before there was a list

zqqw commented 3 years ago
#include <alpm.h>
#include <string.h>

int main()
{
  int rval = 0;
  alpm_errno_t err;
  alpm_handle_t *handle = alpm_initialize("/", "/var/lib/pacman", &err);
  if (!handle) return 1;

  alpm_list_t *myalpmlist = alpm_option_get_architectures(handle);
  if (myalpmlist == NULL) printf ("myalpmlist is null\n");
  myalpmlist = 0;
  alpm_list_add (myalpmlist, strdup("somearch"));
  rval = alpm_option_set_architectures(handle, myalpmlist);
  if (rval != 0) exit (1);
  myalpmlist = alpm_option_get_architectures(handle);
  if (myalpmlist == NULL) printf ("myalpmlist is null\n");
  alpm_release(handle);
}

(gcc -lalpm -o xyz xyz.c) I've been trying some test stuff but not figured it out yet, the above says null both times for example. Also if you pass a char (cstring in Nim) instead of an alpm_list_t as your patch did then it will either do nothing if the string is NULL otherwise it will segfault which could be unfortunate if you were part way through an upgrade somehow :). So for this area my question would be how can you put a char into an alpm_list_t. But I have still not figured whether the value that was previously a string is still one:

pakku/src/wrapper/alpm.nim
template withAlpm*(root: string, db: string, dbs: seq[string], arch: string,           < arch is a string provided by withAlpm
  handle: untyped, alpmDbs: untyped, errors: untyped, body: untyped): untyped =

because if that was now an alpm_list_t then you would simply need to swap the wrapper to the new function name and alter the type from a string to an object or something.

Ckath commented 3 years ago

I dont think you're supposed to manipulate the list like that, the add function is there for that reason and seems to work as expected:

#include <alpm.h>
#include <string.h>

int main()
{
  int rval = 0;
  alpm_errno_t err;
  alpm_handle_t *handle = alpm_initialize("/", "/var/lib/pacman", &err);
  if (!handle) return 1;

  alpm_list_t *myalpmlist = alpm_option_get_architectures(handle);
  if (myalpmlist == NULL) printf ("myalpmlist is null\n");

  alpm_option_add_architecture(handle, "somearch");
  myalpmlist = alpm_option_get_architectures(handle);
  if (myalpmlist == NULL) printf ("myalpmlist is null\n");
  else printf ("myalplist is not null anymore\n");

  alpm_release(handle);
}
./xyz
myalpmlist is null
myalplist is not null anymore

Also if you pass a char* (cstring in Nim) instead of an alpm_list_t as your patch did

I dont do that, as I said the function signature of alpm_option_add_architecture is 1:1 with the old set_arch

zqqw commented 3 years ago

Oh yes, I see. Sorry, I was thinking about set not add as I had been reading about that. The arch string is probably originating from the pacman commandline option --arch and this is most likely going to be used in the same way for aur packages so it won't be affected by the pacman changes and should still be a string.

zqqw commented 3 years ago

Thank you, that is marvelous, merged now. If you come up with any more good ideas they will be welcomed, if cautiously sometimes! Best wishes.