Bug#1041485: RFS: spades/3.15.5+dfsg-2.1 [NMU] [RC] -- genome assembler for single-cell and isolates data sets

X-Debbugs-Cc: [email protected]

hi!

On Thu, Jul 20, 2023 at 2:27 AM Étienne Mollier <[email protected]> wrote:
>
> X-Debbugs-Cc: [email protected]
>
> Hi Bo YU,
>
> Bo YU, on 2023-07-19:
> > I am looking for a sponsor for my package "spades":
> […]
> > Changes since the last upload:
> >
> >  spades (3.15.5+dfsg-2.1) unstable; urgency=medium
> >  .
> >    * Non-maintainer upload.
>
> If you happen to go by the Salsa login vimerbf-guest, then you
> should have direct access to push directly to the team
> maintained repository.  Don't hesitate to work there, and ping
> the debian-med[1] list once you are happy with your changes and
> they are ready for "Team upload.".  ;)
>
Yeah, it is me. I just got access at yesterday so I am not sure I can
follow the workflow of Debian med team. But now I can try after your
confirmation.:)
> >    * Fix ftbfs on gcc-13. (Closes: #1037863)
>
> Thanks for your help fixing that bug, and also for the forward
> upstream.  My only matter for head scratching is the following
> hunk in your patch:
>
> --- a/assembler/ext/src/llvm/Signals.inc
> +++ b/assembler/ext/src/llvm/Signals.inc
> @@ -43,6 +43,7 @@
>  #include "llvm/Support/Program.h"
>  #include "llvm/Support/SaveAndRestore.h"
>  #include "llvm/Support/raw_ostream.h"
> +#include "llvm/Support/Signals.h"
>  #include <algorithm>
>  #include <string>
>  #include <sysexits.h>
>
> Including cstdint would have probably been less intrusive, and
> solves the build failure as far as I could test.  Was there a
> specific reason to include the whole llvm/Support/Signals.h
> instead of just cstdint?
>
Okay, I rebuild the package again without the change, it works.
I recalled the background, maybe it messed up due to previous
error I ignored. In fact, maybe I overlooked the log:
```
[ 46%] Building CXX object ext/llvm/CMakeFiles/llvm-support.dir/StringMap.cpp.o
In file included from /<<PKGBUILDDIR>>/assembler/ext/src/llvm/Signals.cpp:219:
/<<PKGBUILDDIR>>/assembler/ext/src/llvm/Signals.inc:347:50: error:
'void llvm::sys::CleanupOnSignal(uintptr_t)' should have been declared
inside 'llvm::sys'
  347 | void llvm::sys::CleanupOnSignal(uintptr_t Context) {
```

I will clean up the change for here and upstream.

> Note the file is copied from llvm source code, so I suspect that
> whatever the right change is, it may have a non negligible
> contribution in resolving #1037760 affecting llvm-toolchain-15.
>
> > BTW, the package maybe only need to build target like x64.
>
> I guess you are referring to #976523.  In case a package were to
> drop dependency amd64 specific flags and become architecture
> indepent, then this would be automatically caught by the build
> daemons network.  That being said, in the specific case of
> spades, the package looks to already be exluded by the buildd
> network[2]; if that is the case, reintroducing non-amd64
> architectures will require a couple of manual steps anyway.
>
Oh, thanks you told me the background so today I learned many here.

I think it is okay i push the update to the salsa repo directly as
your suggestion.

Thanks again!

BR,
Bo

> [1]: mailto:[email protected]
> [2]: https://buildd.debian.org/status/package.php?p=spades
>
> Have a nice day,  :)
> --
>   .''`.  Étienne Mollier <[email protected]>
>  : :' :  gpg: 8f91 b227 c7d6 f2b1 948c  8236 793c f67e 8f0d 11da
>  `. `'   sent from /dev/pts/1, please excuse my verbosity
>    `-    on air: The Flower Kings - Garden Of Dreams (Part B Ed…