Bug 69745

Summary: [GTK] Fix make distcheck build
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: gustavo, mrobinson, pnormand, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch
none
Updated patch
none
Updated patch
mrobinson: review+
Patch updated and without gtk-doc part mrobinson: review+

Description Carlos Garcia Campos 2011-10-10 00:27:45 PDT
It's currently broken
Comment 1 Carlos Garcia Campos 2011-10-10 00:31:52 PDT
Created attachment 110335 [details]
Patch
Comment 2 Carlos Garcia Campos 2011-10-10 00:41:01 PDT
Created attachment 110336 [details]
Updated patch

I forgot to add makefile and version.xml docs to DISTCLEAN
Comment 3 Xan Lopez 2011-10-10 08:24:24 PDT
Comment on attachment 110336 [details]
Updated patch

View in context: https://bugs.webkit.org/attachment.cgi?id=110336&action=review

> Source/WebKit2/GNUmakefile.am:1400
> +	$(WebKit2)/UIProcess/API/gtk/docs/GNUmakefile.* \

What else do you want to dist other than the .am file?

> Source/WebKit2/GNUmakefile.am:1408
> +	$(top_builddir)/WebKit2/UIProcess/API/gtk/docs/GNUmakefile

Seems really strange that you'd need to explicitly add the makefile to distcleanfiles. I've never seen this done? I suspect there's a better way to fix whatever issue you are having. Can you paste the error message?
Comment 4 Carlos Garcia Campos 2011-10-11 00:01:04 PDT
(In reply to comment #3)
> (From update of attachment 110336 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=110336&action=review
> 
> > Source/WebKit2/GNUmakefile.am:1400
> > +	$(WebKit2)/UIProcess/API/gtk/docs/GNUmakefile.* \
> 
> What else do you want to dist other than the .am file?

The .in too. I don't know why, but make distcheck complained it was missing, then I looked at how wk1 did it and I copied the approach. 

> > Source/WebKit2/GNUmakefile.am:1408
> > +	$(top_builddir)/WebKit2/UIProcess/API/gtk/docs/GNUmakefile
> 
> Seems really strange that you'd need to explicitly add the makefile to distcleanfiles. I've never seen this done?

me neither, the first time I have seen it is in wk1 makefile. 

> I suspect there's a better way to fix whatever issue you are having. 

I guess the problem is that this makefile is not included to the main makefile like all others, but I don't know. 

> Can you paste the error message?

I don't remember the exactly message, but when distcheck was about to finish it complained there were two files still in tree.
Comment 5 Carlos Garcia Campos 2011-10-17 07:52:52 PDT
Created attachment 111263 [details]
Updated patch
Comment 6 Martin Robinson 2011-10-17 07:55:28 PDT
Comment on attachment 111263 [details]
Updated patch

Okay. I'm working on removing the doc makefiles anyway.
Comment 7 WebKit Review Bot 2011-10-17 07:55:34 PDT
Attachment 111263 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit', '--chromium']" exit_code: 2

Updating OpenSource
From git://git.webkit.org/WebKit
   4af16b0..a891ab4  master     -> origin/master
	M	LayoutTests/inspector/debugger/compiler-source-mapping-expected.txt
	M	LayoutTests/inspector/debugger/compiler-source-mapping.html
	M	LayoutTests/ChangeLog
	M	Source/WebCore/ChangeLog
	M	Source/WebCore/inspector/front-end/CompilerSourceMapping.js
r97621 = a891ab4ada0035dd47865ce8b1dd9b1e3136f4e1 (refs/remotes/trunk)
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/trunk.
Updating chromium port dependencies using gclient...
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Re-trying 'depot_tools/gclient sync'
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Re-trying 'depot_tools/gclient sync'
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Error: 'depot_tools/gclient sync' failed 3 tries and returned 256 at Tools/Scripts/update-webkit-chromium line 107.
Re-trying 'depot_tools/gclient sync'
No such file or directory at Tools/Scripts/update-webkit line 104.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Carlos Garcia Campos 2011-10-17 08:02:15 PDT
(In reply to comment #6)
> (From update of attachment 111263 [details])
> Okay. I'm working on removing the doc makefiles anyway.

Thanks, but Xan told me he was not going to approve this patch in its current state, so I won't land it for now until make sure he agrees.
Comment 9 Xan Lopez 2011-10-17 13:52:27 PDT
(In reply to comment #8)
> Thanks, but Xan told me he was not going to approve this patch in its current state, so I won't land it for now until make sure he agrees.

Since we are not releasing today I still think it's a mistake to land this given that no one seems to understand why it works or why it's needed. Gustavo said he'd try to have a look (since he was involved in the previous hack in wk1) and give feedback on the bug. If the makefiles are going to disappear (soon?) anyway I guess I don't care as much, but waiting for Gustavo's opinion still seems more sensible.
Comment 10 Martin Robinson 2011-10-17 13:58:20 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > Thanks, but Xan told me he was not going to approve this patch in its current state, so I won't land it for now until make sure he agrees.
> 
> Since we are not releasing today I still think it's a mistake to land this given that no one seems to understand why it works or why it's needed. Gustavo said he'd try to have a look (since he was involved in the previous hack in wk1) and give feedback on the bug. If the makefiles are going to disappear (soon?) anyway I guess I don't care as much, but waiting for Gustavo's opinion still seems more sensible.

My advice to Carlos was just to just land the obvious bits of this patch and save the gtkdoc stuff for later.
Comment 11 Carlos Garcia Campos 2011-10-18 02:06:54 PDT
(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > Thanks, but Xan told me he was not going to approve this patch in its current state, so I won't land it for now until make sure he agrees.
> > 
> > Since we are not releasing today I still think it's a mistake to land this given that no one seems to understand why it works or why it's needed. Gustavo said he'd try to have a look (since he was involved in the previous hack in wk1) and give feedback on the bug. If the makefiles are going to disappear (soon?) anyway I guess I don't care as much, but waiting for Gustavo's opinion still seems more sensible.
> 
> My advice to Carlos was just to just land the obvious bits of this patch and save the gtkdoc stuff for later.

I can do that, but this bug is about fixing make distcheck, we should open a new bug for the gtk-doc stuff and rename this to something else, because without the gtk-doc stuff this patch doesn't fix distcheck.
Comment 12 Gustavo Noronha (kov) 2011-10-19 16:17:38 PDT
I am proposing streamlining the wk1 docs build in a way that this hack would not be necessary anymore: https://bugs.webkit.org/show_bug.cgi?id=70447

Martin is working on this new way of building docs which seems like it will be superior and easier to use, but if we want a quicker solution in the meantime adapting the wk2 docs build to work on top of the proposed change above might be good. It may also make life easier to packagers like myself =P
Comment 13 Carlos Garcia Campos 2011-10-24 03:16:07 PDT
Created attachment 112170 [details]
Patch updated and without gtk-doc part

Fix some distcheck issues (distcheck still doesn't pass because of the gtk-doc files)
Comment 14 Martin Robinson 2011-10-24 08:58:35 PDT
Comment on attachment 112170 [details]
Patch updated and without gtk-doc part

Thank you!
Comment 15 Carlos Garcia Campos 2011-10-24 09:44:07 PDT
Committed r98250: <http://trac.webkit.org/changeset/98250>