Skip to content
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.

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

Libalpm13 fixups #13

Merged
merged 3 commits into from
Jun 2, 2021
Merged

Libalpm13 fixups #13

merged 3 commits into from
Jun 2, 2021

Conversation

Ckath
Copy link

@Ckath Ckath commented Jun 2, 2021

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.

Ckath added 2 commits June 2, 2021 03:12
alpm_set_arch -> alpm_add_architecture
it seems to do the same thing as the old set_arch
@zqqw
Copy link
Owner

zqqw commented Jun 2, 2021

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
Copy link
Author

Ckath commented Jun 2, 2021

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
Copy link
Owner

zqqw commented Jun 2, 2021

#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
Copy link
Author

Ckath commented Jun 2, 2021

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
Copy link
Owner

zqqw commented Jun 2, 2021

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 zqqw merged commit 334e09a into zqqw:master Jun 2, 2021
@zqqw
Copy link
Owner

zqqw commented Jun 2, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants