GNU bug report logs - #46113
[PATCH] gnu: obs: Update obs to fb347c.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 46113 in the body.
You can then email your comments to 46113 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Andrew Tropin <andrew <at> trop.in>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: obs: Update obs to fb347c.
Date: Tue, 26 Jan 2021 18:59:39 +0300

This version of obs adds support for OBS_PLUGINS*_PATH environment variables,
which are required to be able to load plugins, which are present in profile.

It will make it possible for following packages to work:
http://issues.guix.gnu.org/45961
http://issues.guix.gnu.org/45960

* gnu/packages/video.scm (obs): Update to fb347c.
---
 gnu/packages/video.scm | 116 ++++++++++++++++++++++-------------------
 1 file changed, 63 insertions(+), 53 deletions(-)

diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
index 28cde06f04..0f9b405261 100644
--- a/gnu/packages/video.scm
+++ b/gnu/packages/video.scm
@@ -3076,62 +3076,72 @@ be used for realtime video capture via Linux-specific APIs.")
     (license (list license:lgpl2.1+ license:gpl2))))
 
 (define-public obs
-  (package
-    (name "obs")
-    (version "26.1.0")
-    (source (origin
-              (method git-fetch)
-              (uri (git-reference
-                    (url "https://github.com/obsproject/obs-studio")
-                    (commit version)))
-              (file-name (git-file-name name version))
-              (sha256
-               (base32
-                "0p8wdzm9imn3s17arr206sz92g4pkacfcpfbwvhvgkrrs4w000bx"))))
-    (build-system cmake-build-system)
-    (arguments
-     `(#:configure-flags
-       (list (string-append "-DOBS_VERSION_OVERRIDE=" ,version)
-             "-DENABLE_UNIT_TESTS=TRUE")
-       #:phases
-       (modify-phases %standard-phases
-         (add-after 'install 'wrap-executable
-           (lambda* (#:key outputs #:allow-other-keys)
-             (let ((out (assoc-ref outputs "out"))
-                   (plugin-path (getenv "QT_PLUGIN_PATH")))
-               (wrap-program (string-append out "/bin/obs")
-                 `("QT_PLUGIN_PATH" ":" prefix (,plugin-path))))
-             #t)))))
-    (native-inputs
-     `(("cmocka" ,cmocka)
-       ("pkg-config" ,pkg-config)))
-    (inputs
-     `(("alsa-lib" ,alsa-lib)
-       ("curl" ,curl)
-       ("eudev" ,eudev)
-       ("ffmpeg" ,ffmpeg)
-       ("fontconfig" ,fontconfig)
-       ("freetype" ,freetype)
-       ("jack" ,jack-1)
-       ("jansson" ,jansson)
-       ("libx264" ,libx264)
-       ("libxcomposite" ,libxcomposite)
-       ("mbedtls" ,mbedtls-apache)
-       ("mesa" ,mesa)
-       ("pulseaudio" ,pulseaudio)
-       ("qtbase" ,qtbase)
-       ("qtsvg" ,qtsvg)
-       ("qtx11extras" ,qtx11extras)
-       ("speexdsp" ,speexdsp)
-       ("v4l-utils" ,v4l-utils)
-       ("zlib" ,zlib)))
-    (synopsis "Live streaming software")
-    (description "Open Broadcaster Software provides a graphical interface for
+  (let ((commit "fb347c3c62ced2ea302769e449d300fd923c2d4b")
+        (revision "1"))
+    (package
+      (name "obs")
+      (version (git-version "26.1.2" revision commit))
+      (source (origin
+                (method git-fetch)
+                (uri (git-reference
+                      (url "https://github.com/obsproject/obs-studio")
+                      (commit commit)))
+                (file-name (git-file-name name version))
+                (sha256
+                 (base32
+                  "017llgj1hlfvk2622qa44d8iz6d0kahhckn421dypj09a4n6aajz"))))
+      (build-system cmake-build-system)
+      (arguments
+       `(#:configure-flags
+         (list (string-append "-DOBS_VERSION_OVERRIDE=" ,version)
+               "-DENABLE_UNIT_TESTS=TRUE")
+         #:phases
+         (modify-phases %standard-phases
+           (add-after 'install 'wrap-executable
+             (lambda* (#:key outputs #:allow-other-keys)
+               (let ((out (assoc-ref outputs "out"))
+                     (plugin-path (getenv "QT_PLUGIN_PATH")))
+                 (wrap-program (string-append out "/bin/obs")
+                   `("QT_PLUGIN_PATH" ":" prefix (,plugin-path))))
+               #t)))))
+      (native-inputs
+       `(("cmocka" ,cmocka)
+         ("pkg-config" ,pkg-config)))
+      (inputs
+       `(("alsa-lib" ,alsa-lib)
+         ("curl" ,curl)
+         ("eudev" ,eudev)
+         ("ffmpeg" ,ffmpeg)
+         ("fontconfig" ,fontconfig)
+         ("freetype" ,freetype)
+         ("jack" ,jack-1)
+         ("jansson" ,jansson)
+         ("libx264" ,libx264)
+         ("libxcomposite" ,libxcomposite)
+         ("mbedtls" ,mbedtls-apache)
+         ("mesa" ,mesa)
+         ("pulseaudio" ,pulseaudio)
+         ("qtbase" ,qtbase)
+         ("qtsvg" ,qtsvg)
+         ("qtx11extras" ,qtx11extras)
+         ("speexdsp" ,speexdsp)
+         ("v4l-utils" ,v4l-utils)
+         ("zlib" ,zlib)))
+      (native-search-paths
+       (list
+        (search-path-specification
+         (variable "OBS_PLUGINS_DATA_PATH")
+         (files '("share/obs/obs-plugins")))
+        (search-path-specification
+         (variable "OBS_PLUGINS_PATH")
+         (files '("lib/obs-plugins")))))
+      (synopsis "Live streaming software")
+      (description "Open Broadcaster Software provides a graphical interface for
 video recording and live streaming.  OBS supports capturing audio and video
 from many input sources such as webcams, X11 (for screencasting), PulseAudio,
 and JACK.")
-    (home-page "https://obsproject.com")
-    (license license:gpl2+)))
+      (home-page "https://obsproject.com")
+      (license license:gpl2+))))
 
 (define-public libvdpau
   (package
-- 
2.30.0

Message #8 received at 46113 <at> debbugs.gnu.org (full text, mbox):

From: Andrew Tropin <andrew <at> trop.in>
To: 46113 <at> debbugs.gnu.org
Subject: [PATCH] gnu: obs: Update obs to fb347c.
Date: Tue, 26 Jan 2021 19:26:24 +0300

It's a commit from master branch containing changes from PR:
https://github.com/obsproject/obs-studio/pull/4067

It's just few commits after 26.1.2 and it seems there is no any big
changes since that and should be safe to use.

I asked for 26.1.3 tag, but it seems that it won't happen in nearest
future: https://github.com/obsproject/obs-studio/issues/4100

As one of developers said to me, usually obs releases once in 3-4
months, that is why I decided to use a commit instead of tag.

The changes is needed to be able to specify plugin load path using
native-search-paths to be able to package obs plugins with guix.

Message #11 received at 46113 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: Andrew Tropin <andrew <at> trop.in>
Cc: 46113 <at> debbugs.gnu.org
Subject: Re: [bug#46113] [PATCH] gnu: obs: Update obs to fb347c.
Date: Tue, 26 Jan 2021 16:10:23 -0500

On Tue, Jan 26, 2021 at 06:59:39PM +0300, Andrew Tropin wrote:
> 
> This version of obs adds support for OBS_PLUGINS*_PATH environment variables,
> which are required to be able to load plugins, which are present in profile.
> 
> It will make it possible for following packages to work:
> http://issues.guix.gnu.org/45961
> http://issues.guix.gnu.org/45960

Thanks!

Your patch doesn't apply to the current master branch. Can you rebase it
and send a revision?

Message #14 received at 46113 <at> debbugs.gnu.org (full text, mbox):

From: Alexey Abramov <levenson <at> mmer.org>
To: Andrew Tropin <andrew <at> trop.in>
Cc: 46113 <at> debbugs.gnu.org
Subject: Re: [bug#46113] [PATCH] gnu: obs: Update obs to fb347c.
Date: Wed, 27 Jan 2021 08:27:38 +0100

Hi Adrew,

I patched obs here [1]. I checked the upstream patch, and noticed that it ammends AddExtraModulePaths function. With Guix the list of plugin directories will contain *two* locations with the very same plugins, one with the obs store and guix-profile, hence obs will print 'Duplicate library?' warning messages.

I packaged the main location to avoid such a message.

Footnotes:
[1]  https://issues.guix.gnu.org/45707

-- 
Alexey

Message #17 received at 46113 <at> debbugs.gnu.org (full text, mbox):

From: Andrew Tropin <andrew <at> trop.in>
To: Alexey Abramov <levenson <at> mmer.org>
Cc: 46113 <at> debbugs.gnu.org
Subject: [PATCH] gnu: obs: Update obs to fb347c.
Date: Wed, 27 Jan 2021 12:36:58 +0300

> I patched obs here [1]. I checked the upstream patch, and noticed that
> it ammends AddExtraModulePaths function. With Guix the list of plugin
> directories will contain *two* locations with the very same plugins,
> one with the obs store and guix-profile, hence obs will print
> 'Duplicate library?' warning messages.

It's very true, obs, will have same plugins in load paths twice, but
from what I found the problem will happen only during shutdown of obs,
when it will try to unload the same plugin twice, which doesn't affect
runtime anyhow. I had a small workaround for that (a separate
envirnoment variable, which toggles the loading of plugins from
INSTALL_PREFIX), but during review one of obs mantainers said that it's
an adhoc solution and better to solve this problem in general. The
comment from @kkartaltepe and related changes [fn:1].

The other solution was to check if OBS_PLUGINS_PATH is present and ommit
loading of builtin plugins in that case, but it will break in case
someone wants to specify just additional plugin load path (some non-guix
use case).

I decided to go without any fix to that problem as it is not directly
related to changeset I proposed and doesn't affect runtime, but it's
probably a good idea to report double free on shutdown issue to
upstream.

* Footnotes

[fn:1] https://github.com/obsproject/obs-studio/pull/4067 

Message #20 received at 46113 <at> debbugs.gnu.org (full text, mbox):

From: Andrew Tropin <andrew <at> trop.in>
To: Leo Famulari <leo <at> famulari.name>
Cc: 46113 <at> debbugs.gnu.org
Subject: [PATCH] gnu: obs: Update to fb347c.
Date: Wed, 27 Jan 2021 12:45:30 +0300

This version of obs adds support for OBS_PLUGINS*_PATH environment variables,
which are required to be able to load plugins, which are present in profile.

It will make it possible for following packages to work:
http://issues.guix.gnu.org/45961
http://issues.guix.gnu.org/45960

* gnu/packages/video.scm (obs): Update to fb347c.
---
 gnu/packages/video.scm | 116 ++++++++++++++++++++++-------------------
 1 file changed, 63 insertions(+), 53 deletions(-)

diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
index 97cb7d6837..f9741dc843 100644
--- a/gnu/packages/video.scm
+++ b/gnu/packages/video.scm
@@ -3077,62 +3077,72 @@ be used for realtime video capture via Linux-specific APIs.")
     (license (list license:lgpl2.1+ license:gpl2))))
 
 (define-public obs
-  (package
-    (name "obs")
-    (version "26.1.2")
-    (source (origin
-              (method git-fetch)
-              (uri (git-reference
-                    (url "https://github.com/obsproject/obs-studio")
-                    (commit version)))
-              (file-name (git-file-name name version))
-              (sha256
-               (base32
-                "1k1asqiqw757v59ayx0w029ril947hs0lcp8n91knzjl891fr4nc"))))
-    (build-system cmake-build-system)
-    (arguments
-     `(#:configure-flags
-       (list (string-append "-DOBS_VERSION_OVERRIDE=" ,version)
-             "-DENABLE_UNIT_TESTS=TRUE")
-       #:phases
-       (modify-phases %standard-phases
-         (add-after 'install 'wrap-executable
-           (lambda* (#:key outputs #:allow-other-keys)
-             (let ((out (assoc-ref outputs "out"))
-                   (plugin-path (getenv "QT_PLUGIN_PATH")))
-               (wrap-program (string-append out "/bin/obs")
-                 `("QT_PLUGIN_PATH" ":" prefix (,plugin-path))))
-             #t)))))
-    (native-inputs
-     `(("cmocka" ,cmocka)
-       ("pkg-config" ,pkg-config)))
-    (inputs
-     `(("alsa-lib" ,alsa-lib)
-       ("curl" ,curl)
-       ("eudev" ,eudev)
-       ("ffmpeg" ,ffmpeg)
-       ("fontconfig" ,fontconfig)
-       ("freetype" ,freetype)
-       ("jack" ,jack-1)
-       ("jansson" ,jansson)
-       ("libx264" ,libx264)
-       ("libxcomposite" ,libxcomposite)
-       ("mbedtls" ,mbedtls-apache)
-       ("mesa" ,mesa)
-       ("pulseaudio" ,pulseaudio)
-       ("qtbase" ,qtbase)
-       ("qtsvg" ,qtsvg)
-       ("qtx11extras" ,qtx11extras)
-       ("speexdsp" ,speexdsp)
-       ("v4l-utils" ,v4l-utils)
-       ("zlib" ,zlib)))
-    (synopsis "Live streaming software")
-    (description "Open Broadcaster Software provides a graphical interface for
+  (let ((commit "fb347c3c62ced2ea302769e449d300fd923c2d4b")
+        (revision "1"))
+    (package
+      (name "obs")
+      (version (git-version "26.1.2" revision commit))
+      (source (origin
+                (method git-fetch)
+                (uri (git-reference
+                      (url "https://github.com/obsproject/obs-studio")
+                      (commit version)))
+                (file-name (git-file-name name version))
+                (sha256
+                 (base32
+                  "017llgj1hlfvk2622qa44d8iz6d0kahhckn421dypj09a4n6aajz"))))
+      (build-system cmake-build-system)
+      (arguments
+       `(#:configure-flags
+         (list (string-append "-DOBS_VERSION_OVERRIDE=" ,version)
+               "-DENABLE_UNIT_TESTS=TRUE")
+         #:phases
+         (modify-phases %standard-phases
+           (add-after 'install 'wrap-executable
+             (lambda* (#:key outputs #:allow-other-keys)
+               (let ((out (assoc-ref outputs "out"))
+                     (plugin-path (getenv "QT_PLUGIN_PATH")))
+                 (wrap-program (string-append out "/bin/obs")
+                   `("QT_PLUGIN_PATH" ":" prefix (,plugin-path))))
+               #t)))))
+      (native-inputs
+       `(("cmocka" ,cmocka)
+         ("pkg-config" ,pkg-config)))
+      (inputs
+       `(("alsa-lib" ,alsa-lib)
+         ("curl" ,curl)
+         ("eudev" ,eudev)
+         ("ffmpeg" ,ffmpeg)
+         ("fontconfig" ,fontconfig)
+         ("freetype" ,freetype)
+         ("jack" ,jack-1)
+         ("jansson" ,jansson)
+         ("libx264" ,libx264)
+         ("libxcomposite" ,libxcomposite)
+         ("mbedtls" ,mbedtls-apache)
+         ("mesa" ,mesa)
+         ("pulseaudio" ,pulseaudio)
+         ("qtbase" ,qtbase)
+         ("qtsvg" ,qtsvg)
+         ("qtx11extras" ,qtx11extras)
+         ("speexdsp" ,speexdsp)
+         ("v4l-utils" ,v4l-utils)
+         ("zlib" ,zlib)))
+      (native-search-paths
+       (list
+        (search-path-specification
+         (variable "OBS_PLUGINS_DATA_PATH")
+         (files '("share/obs/obs-plugins")))
+        (search-path-specification
+         (variable "OBS_PLUGINS_PATH")
+         (files '("lib/obs-plugins")))))
+      (synopsis "Live streaming software")
+      (description "Open Broadcaster Software provides a graphical interface for
 video recording and live streaming.  OBS supports capturing audio and video
 from many input sources such as webcams, X11 (for screencasting), PulseAudio,
 and JACK.")
-    (home-page "https://obsproject.com")
-    (license license:gpl2+)))
+      (home-page "https://obsproject.com")
+      (license license:gpl2+))))
 
 (define-public libvdpau
   (package
-- 
2.30.0

Message #23 received at 46113 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: Andrew Tropin <andrew <at> trop.in>
Cc: 46113 <at> debbugs.gnu.org
Subject: Re: [PATCH] gnu: obs: Update to fb347c.
Date: Mon, 1 Feb 2021 19:48:44 -0500

On Wed, Jan 27, 2021 at 12:45:30PM +0300, Andrew Tropin wrote:
> This version of obs adds support for OBS_PLUGINS*_PATH environment variables,
> which are required to be able to load plugins, which are present in profile.
> 
> It will make it possible for following packages to work:
> http://issues.guix.gnu.org/45961
> http://issues.guix.gnu.org/45960
> 
> * gnu/packages/video.scm (obs): Update to fb347c.

Thanks for this!

I'm wondering, instead of building from a Git commit, can we cherry-pick
the commit as a patch?

https://github.com/obsproject/obs-studio/commit/fb347c3c62ced2ea302769e449d300fd923c2d4b.patch

Does it work if we apply that to the 26.1.2 release?

It would be a little more precise.

Message #26 received at 46113 <at> debbugs.gnu.org (full text, mbox):

From: Andrew Tropin <andrew <at> trop.in>
To: Leo Famulari <leo <at> famulari.name>
Cc: 46113 <at> debbugs.gnu.org
Subject: Re: [PATCH] gnu: obs: Update to fb347c.
Date: Tue, 2 Feb 2021 14:39:03 +0300

Hello Leo!

> Does it work if we apply that to the 26.1.2 release?

It should work, originally I wrote a patch for 26.1.2 and later rebased
the commit on master without any conflicts.

> It would be a little more precise.

There was a discussion on irc about it and at the end of the day I would
prefer to use the commit from master branch, rather than maintaining a
separate patch, which we will need to remove later, when 26.1.3 or
another version will be released.

There were not so many changes since 26.1.2 up to the fb347c, I fluently
went through them, they are mostly minor and were accepted after the
pretty strict code review process.

From what I know it should be safe to use this commit, but I can contact
upstream devs and additionally clarify that.

Overall I'm ok with using an exact patch, but prefer using a commit from
master. WDYT?


-- 
Best regards,
Andrew Tropin

Message #29 received at 46113 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: Andrew Tropin <andrew <at> trop.in>
Cc: 46113 <at> debbugs.gnu.org
Subject: Re: [PATCH] gnu: obs: Update to fb347c.
Date: Tue, 2 Feb 2021 14:33:22 -0500

On Tue, Feb 02, 2021 at 02:39:03PM +0300, Andrew Tropin wrote:
> > Does it work if we apply that to the 26.1.2 release?
> 
> It should work, originally I wrote a patch for 26.1.2 and later rebased
> the commit on master without any conflicts.
> 
> > It would be a little more precise.
> 
> There was a discussion on irc about it and at the end of the day I would
> prefer to use the commit from master branch, rather than maintaining a
> separate patch, which we will need to remove later, when 26.1.3 or
> another version will be released.
> 
> There were not so many changes since 26.1.2 up to the fb347c, I fluently
> went through them, they are mostly minor and were accepted after the
> pretty strict code review process.
> 
> From what I know it should be safe to use this commit, but I can contact
> upstream devs and additionally clarify that.
> 
> Overall I'm ok with using an exact patch, but prefer using a commit from
> master. WDYT?

Thanks for explaining the process you went through. Based on that, I
agree that your patch is good as proposed.

The patch file doesn't apply to current master. It looks like some other
changes regarding this plugins problem have been done to the obs package
since your changes were made.

Can you rebase it on your end and re-generate the patch? Or is it still
necessary?

Message #32 received at 46113 <at> debbugs.gnu.org (full text, mbox):

From: Andrew Tropin <andrew <at> trop.in>
To: Leo Famulari <leo <at> famulari.name>
Cc: 46113 <at> debbugs.gnu.org
Subject: Re: [PATCH] gnu: obs: Update to fb347c.
Date: Tue, 2 Feb 2021 22:43:52 +0300

> Thanks for explaining the process you went through. Based on that, I
> agree that your patch is good as proposed.
>
> The patch file doesn't apply to the current master. It looks like some other
> changes regarding this plugins problem have been done to the obs package
> since your changes were made.
>
> Can you rebase it on your end and re-generate the patch? Or is it still
> necessary?

I'll post to the related thread first https://issues.guix.gnu.org/45707
to discuss what we can do about that situation, and will post back here,
when we will reach some agreement.

-- 
Best regards,
Andrew Tropin

Message #35 received at 46113 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Andrew Tropin <andrew <at> trop.in>
Cc: 46113 <at> debbugs.gnu.org, 45960-done <at> debbugs.gnu.org
Subject: Re: bug#45960: [PATCH] gnu: Add obs-spectralizer.
Date: Wed, 03 Feb 2021 11:43:37 +0100

Hi,

Andrew Tropin <andrew <at> trop.in> skribis:

> It won't load until obs updated to the commit with OBS_PLUGINS*_PATH
> variables support [fn:1], but as you said it's future-proof and already
> installs to the correct location.
>
> There is another very similar patch, which adds another obs plugin. You
> probably already aware of it, but here is a link:
> http://issues.guix.gnu.org/45961. Accidentally, it has empty propagated
> inputs and I'm not sure if it will apply to current master at all, but I
> can update it if it's needed.
>
>
> * Footnotes
>
> [fn:1] http://issues.guix.gnu.org/46113

Ah ha!  So on ‘master’, there’s the OBS_PLUGINS_DIRECTORY patch that you
provided earlier, which is why I wrote that the plugins should work.

Should we wait for the next OBS release instead of packaging an
arbitrary commit?

Ludo’.

Message #38 received at 46113 <at> debbugs.gnu.org (full text, mbox):

From: Alexey Abramov <levenson <at> mmer.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 46113 <at> debbugs.gnu.org, 45960-done <at> debbugs.gnu.org,
 Andrew Tropin <andrew <at> trop.in>
Subject: Re: [bug#46113] bug#45960: [PATCH] gnu: Add obs-spectralizer.
Date: Wed, 03 Feb 2021 15:00:53 +0100

Hi,

Thanks for merging my patch =) I am not insist on keeping [1], but I do think that is more cleaner solution. 

With the upstream patch, Obs will be able to extend the list of plugin directories. We are going to add ~/.guix-profile/ to that list, which means Obs will have two different locations with partially different so files. It doesn't look clean to me. 

In addition, [1] doesn't require to have OBS_PLUGINS_DIRECTORY variables. If there is no such, it will run with its defaults.


Footnotes:
[1]  https://issues.guix.gnu.org/45707

-- 
Alexey

Message #41 received at 46113 <at> debbugs.gnu.org (full text, mbox):

From: Andrew Tropin <andrew <at> trop.in>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 46113 <at> debbugs.gnu.org, 45707-done <at> debbugs.gnu.org,
 Alexey Abramov <levenson <at> mmer.org>
Subject: [PATCH] gnu: obs: Update obs to fb347c.
Date: Wed, 03 Feb 2021 17:40:34 +0300

> Should we wait for the next OBS release instead of packaging an
> arbitrary commit?

I'm not in a hurry, but as I explained earlier in this thread [fn:2] it seems
relatively safe for me to use this commit and getting back later to
26.1.3 or 26.2.0 or whatever next release will be.

> I am not insist on keeping [1], but I do think that is more cleaner
> solution.

In terms of implememntation I like that [fn:1] prevents double loading
of plugins, by excluding obs installation dir from "load-path". However
double loading of the same plugin doesn't seem to break anything. Also,
OBS_PLUGINS_DIRECTORY variable name maybe a little better than
OBS_PLUGINS_PATH as it contains only one path.

The problem is that now there are two almost identical mechanisms (one
in upstream and one via patch [fn:1]), which can bring some maintanance
problems in the future.

There are two good option in my opinion:
- contribute patch from [fn:1] to upstream (reverting OBS_PLUGINS_PATH)
- revert [fn:1] and use OBS_PLUGINS_PATH from upstream

If Alexey ready to contibute OBS_PLUGINS_DIRECTORY patch to obs
(reverting OBS_PLUGINS_PATH), I would be glad to support it. Otherwise,
I would prefer to revert [fn:1] and apply this one. To prevent
maintanance problems in the future.

* Footnotes

[fn:2] http://issues.guix.gnu.org/46113 

[fn:1] https://issues.guix.gnu.org/45707 

--
Best regards,
Andrew Tropin

Message #44 received at 46113 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Andrew Tropin <andrew <at> trop.in>
Cc: 46113 <at> debbugs.gnu.org, 45707-done <at> debbugs.gnu.org,
 Alexey Abramov <levenson <at> mmer.org>
Subject: Re: [PATCH] gnu: obs: Update obs to fb347c.
Date: Wed, 03 Feb 2021 22:11:26 +0100

Hi!

Andrew Tropin <andrew <at> trop.in> skribis:

> The problem is that now there are two almost identical mechanisms (one
> in upstream and one via patch [fn:1]), which can bring some maintanance
> problems in the future.
>
> There are two good option in my opinion:
> - contribute patch from [fn:1] to upstream (reverting OBS_PLUGINS_PATH)
> - revert [fn:1] and use OBS_PLUGINS_PATH from upstream
>
> If Alexey ready to contibute OBS_PLUGINS_DIRECTORY patch to obs
> (reverting OBS_PLUGINS_PATH), I would be glad to support it. Otherwise,
> I would prefer to revert [fn:1] and apply this one. To prevent
> maintanance problems in the future.

I think it’s nicer to follow upstream in general, but I also don’t mind
using our OBS_PLUGINS_DIRECTORY patch until we upgrade to the next
release.

So, Andrew and Alexey: lemme know and I’ll apply what you consider best!

Ludo’.

Message #47 received at 46113 <at> debbugs.gnu.org (full text, mbox):

From: Alexey Abramov <levenson <at> mmer.org>
To: Andrew Tropin <andrew <at> trop.in>
Cc: 46113 <at> debbugs.gnu.org, 45707-done <at> debbugs.gnu.org
Subject: Re: [PATCH] gnu: obs: Update obs to fb347c.
Date: Thu, 04 Feb 2021 11:45:46 +0100

Hi Andrew,

Andrew Tropin <andrew <at> trop.in> writes:

>> Should we wait for the next OBS release instead of packaging an
>> arbitrary commit?
>
> I'm not in a hurry, but as I explained earlier in this thread [fn:2] it seems
> relatively safe for me to use this commit and getting back later to
> 26.1.3 or 26.2.0 or whatever next release will be.
>
>> I am not insist on keeping [1], but I do think that is more cleaner
>> solution.
>
> In terms of implememntation I like that [fn:1] prevents double loading
> of plugins, by excluding obs installation dir from "load-path". However
> double loading of the same plugin doesn't seem to break anything. Also,
> OBS_PLUGINS_DIRECTORY variable name maybe a little better than
> OBS_PLUGINS_PATH as it contains only one path.
>
> The problem is that now there are two almost identical mechanisms (one
> in upstream and one via patch [fn:1]), which can bring some maintanance
> problems in the future.
>
> There are two good option in my opinion:
> - contribute patch from [fn:1] to upstream (reverting OBS_PLUGINS_PATH)

I would go with this one. But the thing is that [fn:1] is specific and
makes sense only for guix. At least from my point of view. So I doubt
that the upstream accept it. But you can try for sure.

> - revert [fn:1] and use OBS_PLUGINS_PATH from upstream
>
> If Alexey ready to contibute OBS_PLUGINS_DIRECTORY patch to obs
> (reverting OBS_PLUGINS_PATH), I would be glad to support it. Otherwise,
> I would prefer to revert [fn:1] and apply this one. To prevent
> maintanance problems in the future.

As I am not a regular obs user, I am afraid I won't be able to find time
for this soon. Feel free to use/delete/revert [fn:1]. It is your call.

> * Footnotes
>
> [fn:2] http://issues.guix.gnu.org/46113 
>
> [fn:1] https://issues.guix.gnu.org/45707 
>
> --
> Best regards,
> Andrew Tropin

-- 
Alexey

Message #52 received at 46113-done <at> debbugs.gnu.org (full text, mbox):

From: Andrew Tropin <andrew <at> trop.in>
To: Alexey Abramov <levenson <at> mmer.org>
Cc: 46113-done <at> debbugs.gnu.org, 45707-done <at> debbugs.gnu.org
Subject: Re: [PATCH] gnu: obs: Update obs to fb347c.
Date: Thu, 4 Feb 2021 15:49:39 +0300

> > There are two good option in my opinion:
> > - contribute patch from [fn:1] to upstream (reverting OBS_PLUGINS_PATH)

> I would go with this one. But the thing is that [fn:1] is specific and
> makes sense only for guix. At least from my point of view. So I doubt
> that the upstream accept it. But you can try for sure.

> As I am not a regular obs user, I am afraid I won't be able to find
> time for this soon. Feel free to use/delete/revert [fn:1]. It is your
> call.

Thank you for the response!

Probably I won't find time to contribute a patch from [fn:1] to obs any
time soon too. So I will wait for next obs release and will update
package definition to use upstream solution once a new version is
released.

Thank you for your work, sorry for inconvenience)

--
Best regards,
Andrew Tropin

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.