After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 755525 - improve `make dist`
improve `make dist`
Status: RESOLVED FIXED
Product: gstreamer-vaapi
Classification: Other
Component: general
git master
Other Linux
: Normal enhancement
: ---
Assigned To: gstreamer-vaapi maintainer(s)
gstreamer-vaapi maintainer(s)
Depends on: 754087
Blocks: 750547
 
 
Reported: 2015-09-24 10:23 UTC by Víctor Manuel Jáquez Leal
Modified: 2015-11-25 13:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
build: set DISTCHECK_CONFIGURE_FLAGS (1003 bytes, patch)
2015-09-24 10:44 UTC, Víctor Manuel Jáquez Leal
none Details | Review
build: declare real built files (1.73 KB, patch)
2015-09-24 10:44 UTC, Víctor Manuel Jáquez Leal
none Details | Review
build: declare real built files (2.65 KB, patch)
2015-11-18 20:01 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
build: libvpx: update the sources lists (25.97 KB, patch)
2015-11-18 20:02 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
build: set DISTCHECK_CONFIGURE_FLAGS (1003 bytes, patch)
2015-11-18 20:02 UTC, Víctor Manuel Jáquez Leal
none Details | Review
build: declare correctly parse lib built files (2.34 KB, patch)
2015-11-25 10:23 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
build: add conditionally gsth265parse patches (2.52 KB, patch)
2015-11-25 10:23 UTC, Víctor Manuel Jáquez Leal
committed Details | Review

Description Víctor Manuel Jáquez Leal 2015-09-24 10:23:44 UTC
Right now, making a gstreamer-vaapi release is a bit cumbersome, and it could be improved fixing the deb.upstream and the distcheck targets in the make file.

Right now they fail if no previous compilation/installation are executed. The should run smoothly.
Comment 1 sreerenj 2015-09-24 10:33:19 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #0)
> Right now, making a gstreamer-vaapi release is a bit cumbersome, and it
> could be improved fixing the deb.upstream and the distcheck targets in the
> make file.
> 
> Right now they fail if no previous compilation/installation are executed.
> The should run smoothly.

I also had some issues, and was manually setting some environmental variables too ;)
Comment 2 Víctor Manuel Jáquez Leal 2015-09-24 10:44:27 UTC
Created attachment 312036 [details] [review]
build: set DISTCHECK_CONFIGURE_FLAGS

Set the configure flags for distcheck target

Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>
Comment 3 Víctor Manuel Jáquez Leal 2015-09-24 10:44:33 UTC
Created attachment 312037 [details] [review]
build: declare real built files

When runnig the make dist target from a clean tree, it fails because
if could not find the copied files from codecparsers submodule.

They weren't copied because they weren't declared as built sources.

This patch removes the stamp mechanism and use the actual file list to copy
as the built sources.

Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>
Comment 4 Víctor Manuel Jáquez Leal 2015-09-25 09:54:43 UTC
Currently I see a problem when running the dist target on the documentation (gtk-doc).

The current gtk-doc.make expects that the library dependencies are already installed, so it can run the introspection. But it the case of gstreamer plugins, this is not possible.

The gstreamer project solves this with a custom gtk-doc-plugins.make that lives in their 'common' submodule:

http://cgit.freedesktop.org/gstreamer/common/tree/gtk-doc-plugins.mak

Shall we adopt the common submodule too? Or just bring what we need and sync them manually if needed?
Comment 5 Víctor Manuel Jáquez Leal 2015-10-28 18:16:16 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #4)
> 
> Shall we adopt the common submodule too? Or just bring what we need and sync
> them manually if needed?

I'm leaning to import the common submodule, hence gstreamer-vaapi could be closer to a "normal" gstreamer project.
Comment 6 sreerenj 2015-11-17 22:18:52 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #5)
> (In reply to Víctor Manuel Jáquez Leal from comment #4)
> > 
> > Shall we adopt the common submodule too? Or just bring what we need and sync
> > them manually if needed?
> 
> I'm leaning to import the common submodule, hence gstreamer-vaapi could be
> closer to a "normal" gstreamer project.

How about postponing it until we start development in upstream repository??
Comment 7 sreerenj 2015-11-17 23:01:42 UTC
Review of attachment 312036 [details] [review]:

::: Makefile.am
@@ +15,3 @@
+DISTCHECK_CONFIGURE_FLAGS=--enable-gtk-doc --enable-glx --enable-egl \
+	--enable-builtin-libvpx=yes --enable-builtin-codecparsers=yes \
+	--enable-builtin-videoparsers=yes

Why do we specify these flags explicitly ? 
Also this doesn't have all the options..
Comment 8 sreerenj 2015-11-17 23:38:19 UTC
Review of attachment 312037 [details] [review]:

this lgtm
Comment 9 Víctor Manuel Jáquez Leal 2015-11-18 10:28:49 UTC
(In reply to sreerenj from comment #7)
> Review of attachment 312036 [details] [review] [review]:
> 
> ::: Makefile.am
> @@ +15,3 @@
> +DISTCHECK_CONFIGURE_FLAGS=--enable-gtk-doc --enable-glx --enable-egl \
> +	--enable-builtin-libvpx=yes --enable-builtin-codecparsers=yes \
> +	--enable-builtin-videoparsers=yes
> 
> Why do we specify these flags explicitly ? 
> Also this doesn't have all the options..

Yeah, those are an arbitrary list of flags. They are the ones I use to test, so I put them for the distcheck.

But still, distcheck is broken because, among other thinks, the gtk-doc generation doesn't work out-of-the-box.
Comment 10 Víctor Manuel Jáquez Leal 2015-11-18 10:29:51 UTC
I stopped to work on this bug since I found out that the real fix relies on using the common/ submodule from gstreamer.
Comment 11 Víctor Manuel Jáquez Leal 2015-11-18 15:43:29 UTC
(In reply to sreerenj from comment #8)
> Review of attachment 312037 [details] [review] [review]:
> 
> this lgtm

I was to push this patch, but just found that it breaks the parallel compilation. So let's put it back on need-works.
Comment 12 Víctor Manuel Jáquez Leal 2015-11-18 20:01:53 UTC
Created attachment 315852 [details] [review]
build: declare real built files

When runnig the `make dist` target from a clean tree, it fails because
if could not find the copied files from codecparsers submodule.

They weren't copied because they weren't declared as built sources.

This patch removes the stamp mechanism and use the actual file list to copy
as the built sources. Also it fixes the duplication of the parser files.

Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>
Comment 13 Víctor Manuel Jáquez Leal 2015-11-18 20:02:00 UTC
Created attachment 315853 [details] [review]
build: libvpx: update the sources lists

`make dist` broke since commit f06798 (libvpx: Update the submodule to
libvpx-1.4.0) because the sources.frag does not contain all the module
sources.

This patch updates thoroughly the sources.

Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>
Comment 14 Víctor Manuel Jáquez Leal 2015-11-18 20:02:05 UTC
Created attachment 315854 [details] [review]
build: set DISTCHECK_CONFIGURE_FLAGS

Set the configure flags for distcheck target

Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>
Comment 15 Víctor Manuel Jáquez Leal 2015-11-19 10:24:47 UTC
Let's focus first in `make dist` target, since is a first step to fix `make distcheck`, which is completely broken.


Said that, I would like to spark a discussion about distributing libvpx. Shall we do that?

In attachment 315853 [details] [review] I added all the sources for its distribution in the gstreamer-vaapi tarball. Perhaps we could trim it to only the sources the project uses (for example, remove the ARM or MIPS related sources). But I don't know if we could do that (is a BSD 3-clause).
Comment 16 Víctor Manuel Jáquez Leal 2015-11-19 10:25:51 UTC
Comment on attachment 315854 [details] [review]
build: set DISTCHECK_CONFIGURE_FLAGS

Let's mark this as obsolete, since it doesn't belong anymore to this bug report. It should be moved to a bug related with `make distcheck`
Comment 17 Víctor Manuel Jáquez Leal 2015-11-19 10:33:32 UTC
Comment on attachment 315852 [details] [review]
build: declare real built files

I feel comfortable with the current patch. I'm going to push it now.
Comment 18 Víctor Manuel Jáquez Leal 2015-11-19 10:41:15 UTC
Comment on attachment 315852 [details] [review]
build: declare real built files

Attachment 315852 [details] pushed as fc8a0d1 - build: declare real built files
Comment 19 sreerenj 2015-11-19 12:02:25 UTC
Review of attachment 315853 [details] [review]:

Thanks for this fix!
Please push...
Comment 20 Víctor Manuel Jáquez Leal 2015-11-19 15:11:34 UTC
Attachment 315853 [details] pushed as f355e62 - build: libvpx: update the sources lists
Comment 21 sreerenj 2015-11-24 16:26:27 UTC
Reopening this since the attachement1 (build: declare real built files ) is causing more issues.

--generate the tarball (make deb.upstream or make dist)
-- untar the source 
-- make
It will end up with this message:
Reversed (or previously applied) patch detected!  Assume -R? [n] y
Comment 22 sreerenj 2015-11-24 17:30:20 UTC
I am leaning to revert this patch,,, okay?
Comment 23 Víctor Manuel Jáquez Leal 2015-11-25 08:54:03 UTC
(In reply to sreerenj from comment #22)
> I am leaning to revert this patch,,, okay?

For time pressure, I'm OK on reverting it.

But still, the patch is on the right direction. It is a pity that I didn't verify it completely.
Comment 24 Víctor Manuel Jáquez Leal 2015-11-25 10:01:30 UTC
Sree, wait. I think I have a patch. Just a sec.
Comment 25 Víctor Manuel Jáquez Leal 2015-11-25 10:23:29 UTC
Created attachment 316225 [details] [review]
build: declare correctly parse lib built files

This is a continuation of commit fc8a0d12

When declaring BUILT_SOURCES, those files should not be distributed. This
patch avoids the distribution of the generated source code.

Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>
Comment 26 Víctor Manuel Jáquez Leal 2015-11-25 10:23:35 UTC
Created attachment 316226 [details] [review]
build: add conditionally gsth265parse patches

As gsth265parse was added in GStreamer 1.4, and gstreamer-vaapi still support
GStreamer 1.2, the patching of gsth265parse must be conditional to the target
GStreamer version.

Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>
Comment 27 sreerenj 2015-11-25 13:21:03 UTC
Review of attachment 316225 [details] [review]:

pushed, thanks for the  fix
Comment 28 sreerenj 2015-11-25 13:21:20 UTC
Review of attachment 316226 [details] [review]:

pushed, thanks for the  fix