Skip to content

sdsalloc.h doesn't seem to be installed as part of regular hiredis #158

@mcepl

Description

@mcepl

#include <hiredis/sdsalloc.h>

When I look at the Makefile of hiredis:

https://github.com/redis/hiredis/blob/6f5bae8c6900e051da6e677756508707565ce56e/Makefile#L302-L310

I don’t see sdsalloc.h to be installed at all, therefore it is not installed on my openSUSE system. Any ideas how to get this file? Is it a bug in hiredis-py or in the installation Makefile of hiredis?

Activity

chayim

chayim commented on Feb 23, 2023

@chayim
Contributor

@mcepl Have you seen vendor/sdsalloc.h in the codebase? You need to ensure you update submodules as well.

git submodule update --init --recursive is your friend

mcepl

mcepl commented on Feb 23, 2023

@mcepl
Author

@mcepl Have you seen vendor/sdsalloc.h in the codebase? You need to ensure you update submodules as well.

git submodule update --init --recursive is your friend

Given I am trying to use tarball from https://files.pythonhosted.org/packages/source/h/hiredis/hiredis-2.2.2.tar.gz I would hope it is included. It actually is, but I would prefer if I could use include files from the hiredis package itself, not the vendored one.

apteryks

apteryks commented on Mar 18, 2023

@apteryks
Contributor

I'm in the same situation, trying to update this package to 2.2.2 on GNU Guix; there's no sdalloc.h file in hiredis 1.1.1:

$ find /gnu/store/0clwbra3j29m97rajyi5y83lm29s2636-hiredis-1.1.0/include/
/gnu/store/0clwbra3j29m97rajyi5y83lm29s2636-hiredis-1.1.0/include/
/gnu/store/0clwbra3j29m97rajyi5y83lm29s2636-hiredis-1.1.0/include/hiredis
/gnu/store/0clwbra3j29m97rajyi5y83lm29s2636-hiredis-1.1.0/include/hiredis/read.h
/gnu/store/0clwbra3j29m97rajyi5y83lm29s2636-hiredis-1.1.0/include/hiredis/async.h
/gnu/store/0clwbra3j29m97rajyi5y83lm29s2636-hiredis-1.1.0/include/hiredis/alloc.h
/gnu/store/0clwbra3j29m97rajyi5y83lm29s2636-hiredis-1.1.0/include/hiredis/sockcompat.h
/gnu/store/0clwbra3j29m97rajyi5y83lm29s2636-hiredis-1.1.0/include/hiredis/adapters
/gnu/store/0clwbra3j29m97rajyi5y83lm29s2636-hiredis-1.1.0/include/hiredis/adapters/ae.h
/gnu/store/0clwbra3j29m97rajyi5y83lm29s2636-hiredis-1.1.0/include/hiredis/adapters/glib.h
/gnu/store/0clwbra3j29m97rajyi5y83lm29s2636-hiredis-1.1.0/include/hiredis/adapters/ivykis.h
/gnu/store/0clwbra3j29m97rajyi5y83lm29s2636-hiredis-1.1.0/include/hiredis/adapters/libev.h
/gnu/store/0clwbra3j29m97rajyi5y83lm29s2636-hiredis-1.1.0/include/hiredis/adapters/libevent.h
/gnu/store/0clwbra3j29m97rajyi5y83lm29s2636-hiredis-1.1.0/include/hiredis/adapters/libhv.h
/gnu/store/0clwbra3j29m97rajyi5y83lm29s2636-hiredis-1.1.0/include/hiredis/adapters/libuv.h
/gnu/store/0clwbra3j29m97rajyi5y83lm29s2636-hiredis-1.1.0/include/hiredis/adapters/macosx.h
/gnu/store/0clwbra3j29m97rajyi5y83lm29s2636-hiredis-1.1.0/include/hiredis/adapters/poll.h
/gnu/store/0clwbra3j29m97rajyi5y83lm29s2636-hiredis-1.1.0/include/hiredis/adapters/qt.h
/gnu/store/0clwbra3j29m97rajyi5y83lm29s2636-hiredis-1.1.0/include/hiredis/hiredis.h
/gnu/store/0clwbra3j29m97rajyi5y83lm29s2636-hiredis-1.1.0/include/hiredis/sds.h

I find it curious that a 'vendored' file doesn't exist upstream; is it something added by hiredis-py?

I would also like to be able to simply link against a system-provided hiredis.

apteryks

apteryks commented on Mar 18, 2023

@apteryks
Contributor

The https://github.com/redis/hiredis/blob/v1.1.0/sdsalloc.h file seems to contain 3 re-definitions, perhaps for backward compatibility purpose? hiredis-py could probably be adjusted to use alloc.h and the originally named definitions it contains instead.

added a commit that references this issue on Mar 18, 2023
c2a2069
added a commit that references this issue on Mar 19, 2023
7b3c8a3
added a commit that references this issue on Mar 19, 2023
dacfaf1
apteryks

apteryks commented on Mar 19, 2023

@apteryks
Contributor

@mcepl hi! you may be interested in #159 and #161 to allow using a system-provided hiredis library.

mcepl

mcepl commented on Mar 19, 2023

@mcepl
Author
chayim

chayim commented on Jun 5, 2023

@chayim
Contributor

@apteryks where did you get a hiredis 1.1.1 - the last release I see is 1.1.0

bugant

bugant commented on Dec 4, 2023

@bugant

@chayim is there any interest in having #159 and #161 merged? I'm asking because I'm in the process of creating a Fedora package for this project and having the same issue described in here.

reopened this on Jul 11, 2024
gerzse

gerzse commented on Jul 11, 2024

@gerzse

I merged #159 for now.

I tried #161 locally, on Ubuntu 22.04.4 LTS, and the hiredis system header files are so old that the build breaks, it does not see some of the REDIS_REPLY_* definitions. Not sure how to handle this? It can be left in the responsibility of the one who's invoking the build, I guess.

added a commit that references this issue on Jul 11, 2024
f4dd081
apteryks

apteryks commented on Sep 12, 2024

@apteryks
Contributor

Typically build systems would probe for the version available, set some flag, then condition the code based on that flag. Not sure something can be done like this in the Python world though, at least when working with the basic setup.py or newer toml project build systems.

apteryks

apteryks commented on Sep 12, 2024

@apteryks
Contributor

@apteryks where did you get a hiredis 1.1.1 - the last release I see is 1.1.0

It was a typo; I had tested with 1.1.0 indeed.

paulwouters

paulwouters commented on Sep 13, 2024

@paulwouters

I can fix this upstream but the only thing sdsalloc.h contains is:

#include "alloc.h"

#define s_malloc hi_malloc
#define s_realloc hi_realloc
#define s_free hi_free

Maybe just use that instead of trying to include sdsalloc.h ?

apteryks

apteryks commented on Sep 22, 2024

@apteryks
Contributor

That seems easy in isolation but if you force all downstream users to do this, the effort/pain adds up :-).

For this reason I'd favor having that properly resolved upstream.

Thanks for getting back!

paulwouters

paulwouters commented on Sep 23, 2024

@paulwouters

My comment was on a wrong PR, I thought this was our in-house PR, sorry :)

I do think redis should fix that, but I would also be happy to have fedora just install the file if upstream won't

added a commit that references this issue on Apr 24, 2025
ecf8478
uglide

uglide commented on Apr 25, 2025

@uglide
Contributor

I'm closing it as the original issue with sdsalloc.h was fixed. @apteryks let's continue the discussion in your PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Participants

      @mcepl@bugant@chayim@uglide@gerzse

      Issue actions

        sdsalloc.h doesn't seem to be installed as part of regular hiredis · Issue #158 · redis/hiredis-py