78177 2012-02-08 16:27:51 -0800 [GTK] doc rebasing does not respect DESTDIR 2012-02-08 18:41:05 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 gustavo gustavo mrobinson oldest_to_newest 552611 0 gustavo 2012-02-08 16:27:51 -0800 [GTK] doc rebasing does not respect DESTDIR 552613 1 126183 gustavo 2012-02-08 16:30:35 -0800 Created attachment 126183 Patch 552618 2 126183 mrobinson 2012-02-08 16:35:53 -0800 Comment on attachment 126183 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=126183&action=review > Tools/gtk/generate-gtkdoc:113 > + 'destdir' : '', You aren't actually passing the command-line argument here...is that on purpose? 552671 3 126200 gustavo 2012-02-08 17:27:58 -0800 Created attachment 126200 Patch 552683 4 gustavo 2012-02-08 17:38:09 -0800 > > Tools/gtk/generate-gtkdoc:113 > > + 'destdir' : '', > > You aren't actually passing the command-line argument here...is that on purpose? FWIW, I thought that was a default. I thought I had verified it worked, but apparently I did something stupid and my verification was flawed, this couldn't possibly work. The new patch works for real, though =). 552700 5 126200 mrobinson 2012-02-08 17:51:01 -0800 Comment on attachment 126200 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=126200&action=review > Tools/gtk/generate-gtkdoc:54 > + # We should consider using an arguments parsing library if > + # we need more of these complex ones. Better make this a TODO: > Tools/gtk/generate-gtkdoc:68 > + destdir = '' > + use_next = False > + for arg in sys.argv: > + if use_next: > + destdir = arg > + break > + > + if arg.startswith('--destdir'): > + if '=' in arg: > + destdir = arg.split('=')[1] > + break > + else: > + use_next = True > + I think it's fine to be less permissive here for now and then come back later and add real argument handling: for argument in sys.argv[i]: if argument.startswith('--destdir='): destdir = argument.split('=')[1] break > Tools/gtk/generate-gtkdoc:73 > + 'destdir' : destdir, You should add some documentation for this new parameter to gtkdoc.py. > Tools/gtk/gtkdoc.py:359 > - html_dir = os.path.join(self.prefix, 'share', 'gtk-doc', 'html', self.module_name) > + html_dir = os.path.join(self.destdir + self.prefix, 'share', 'gtk-doc', 'html', self.module_name) Hrm. Something seems amiss here. This the only place where you concatenat self.destdir and self.prefix so how could the direcdtory exist already? Perhaps I simply am not understanding this bit of code. 552712 6 gustavo 2012-02-08 17:59:24 -0800 (In reply to comment #5) > (From update of attachment 126200 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=126200&action=review > > > Tools/gtk/generate-gtkdoc:54 > > + # We should consider using an arguments parsing library if > > + # we need more of these complex ones. > > Better make this a TODO: [...] > I think it's fine to be less permissive here for now and then come back later and add real argument handling: > > for argument in sys.argv[i]: > if argument.startswith('--destdir='): > destdir = argument.split('=')[1] > break Yeah, I agree. > > Tools/gtk/generate-gtkdoc:73 > > + 'destdir' : destdir, > > You should add some documentation for this new parameter to gtkdoc.py. Will do. > > Tools/gtk/gtkdoc.py:359 > > - html_dir = os.path.join(self.prefix, 'share', 'gtk-doc', 'html', self.module_name) > > + html_dir = os.path.join(self.destdir + self.prefix, 'share', 'gtk-doc', 'html', self.module_name) > > Hrm. Something seems amiss here. This the only place where you concatenat self.destdir and self.prefix so how could the direcdtory exist already? Perhaps I simply am not understanding this bit of code. The directory is created by the make install hook, right here: http://trac.webkit.org/browser/trunk/Tools/GNUmakefile.am#L297 We only run generate-gtkdoc with --rebase after make install has run, so the directory is guaranteed to exist. 552718 7 126200 mrobinson 2012-02-08 18:05:58 -0800 Comment on attachment 126200 Patch Looks good, though I think I managed to convince you about that --destdir should be renamed to something like --virtual-root or --staging-dir. 552736 8 126200 gustavo 2012-02-08 18:40:52 -0800 Comment on attachment 126200 Patch Landed http://trac.webkit.org/changeset/107164 126183 2012-02-08 16:30:35 -0800 2012-02-08 17:27:52 -0800 Patch bug-78177-20120208222914.patch text/plain 2580 gustavo U3VidmVyc2lvbiBSZXZpc2lvbjogMTA3MTUwCmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggNzNlNWUyMWRiN2E4OTVmODQ5ZGU0NmIzNjIzYTA1ZmQ2 OTkzZTlkOS4uMmJjNzYwMzE5YWQ4MDY4ZDNiMDNlMWU0YzJkY2Q1ODZhMDFhMzNiYiAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDIw IEBACiAyMDEyLTAyLTA4ICBHdXN0YXZvIE5vcm9uaGEgU2lsdmEgIDxnbnNAZ25vbWUub3JnPgog CisgICAgICAgIFtHVEtdIGRvYyByZWJhc2luZyBkb2VzIG5vdCByZXNwZWN0IERFU1RESVIKKyAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTc4MTc3CisKKyAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBHTlVtYWtlZmls ZS5hbTogUGFzcyBERVNURElSIG9uIHRvIGdlbmVyYXRlLWd0a2RvYywgd2hlbgorICAgICAgICBj YWxsaW5nIGl0IGZvciByZWJhc2luZy4KKyAgICAgICAgKiBndGsvZ2VuZXJhdGUtZ3RrZG9jOgor ICAgICAgICAoZ2V0X3dlYmtpdDFfb3B0aW9ucyk6IEhhbmRsZSB0aGUgbmV3IC0tZGVzdGRpciBv cHRpb24uCisgICAgICAgICogZ3RrL2d0a2RvYy5weToKKyAgICAgICAgKEdUS0RvYy5yZWJhc2Vf aW5zdGFsbGVkX2RvY3MpOiBJZiBhIGRlc3RkaXIgaGFzIGJlZW4gZ2l2ZW4sIHBhc3MKKyAgICAg ICAgaXQgb24gdG8gZ3RrZG9jLXJlYmFzZS4KKworMjAxMi0wMi0wOCAgR3VzdGF2byBOb3Jvbmhh IFNpbHZhICA8Z25zQGdub21lLm9yZz4KKwogICAgICAgICBVbnJldmlld2VkIHR5cG8gZml4IHRo YXQgbWFrZXMgZG9jcyBidWlsZCBhZ2FpbiBmb3IgdGhlIGd0azItYmFzZWQKICAgICAgICAgbGli cmFyeS4KIApkaWZmIC0tZ2l0IGEvVG9vbHMvR05VbWFrZWZpbGUuYW0gYi9Ub29scy9HTlVtYWtl ZmlsZS5hbQppbmRleCAyM2Q4MTEyYjliNTBlODQzNDA5YzZjMmFhYWEyYzE1OGZkZmY1ODYxLi45 NDBkZGE3YjVmZmNkNTlhOGM1MTE3N2JhNzJkNDdlNGVhMGI0NGQyIDEwMDY0NAotLS0gYS9Ub29s cy9HTlVtYWtlZmlsZS5hbQorKysgYi9Ub29scy9HTlVtYWtlZmlsZS5hbQpAQCAtMzI2LDcgKzMy Niw3IEBAIGlmIEVOQUJMRV9XRUJLSVQyCiAJICBmaTsgXAogCWZpCiBlbmRpZgotCSQoc3JjZGly KS9Ub29scy9ndGsvZ2VuZXJhdGUtZ3RrZG9jIC0tcmViYXNlCisJJChzcmNkaXIpL1Rvb2xzL2d0 ay9nZW5lcmF0ZS1ndGtkb2MgLS1yZWJhc2UgLS1kZXN0ZGlyPSQke0RFU1RESVJ9CiAKIHVuaW5z dGFsbC1sb2NhbDoKIAlARE9DX01PRFVMRV9WRVJTSU9OPWBjYXQgLi9Eb2N1bWVudGF0aW9uL3dl YmtpdGd0ay92ZXJzaW9uLnhtbGA7IFwKZGlmZiAtLWdpdCBhL1Rvb2xzL2d0ay9nZW5lcmF0ZS1n dGtkb2MgYi9Ub29scy9ndGsvZ2VuZXJhdGUtZ3RrZG9jCmluZGV4IDk5NmI5MWEwMjQyNWEyNTUw YWRiZGFkNDM2NDcyMzg1ZmI0MWE4ZDcuLmFlMmI4YWY0YzkwZDAyMjMxNzYxMTgyOGQ2MmM2ODc5 ZDg2MGEzZDUgMTAwNzU1Ci0tLSBhL1Rvb2xzL2d0ay9nZW5lcmF0ZS1ndGtkb2MKKysrIGIvVG9v bHMvZ3RrL2dlbmVyYXRlLWd0a2RvYwpAQCAtMTEwLDYgKzExMCw3IEBAIGRlZiBnZXRfd2Via2l0 MV9vcHRpb25zKGd0a192ZXJzaW9uKToKICAgICBvcHRpb25zID0gZ2V0X2NvbW1vbl9vcHRpb25z KCkuY29weSgpCiAgICAgb3B0aW9ucy51cGRhdGUoewogICAgICAgICAnbW9kdWxlX25hbWUnIDog J3dlYmtpdGd0aycsCisgICAgICAgICdkZXN0ZGlyJyA6ICcnLAogICAgICAgICAnZG9jX2Rpcicg OiBzcmNfcGF0aCgnZG9jcycpLAogICAgICAgICAnb3V0cHV0X2RpcicgOiBjb21tb24uYnVpbGRf cGF0aCgnRG9jdW1lbnRhdGlvbicsICd3ZWJraXRndGsnKSwKICAgICAgICAgJ3NvdXJjZV9kaXJz JyA6IFtzcmNfcGF0aCgnd2Via2l0JyksIGNvbW1vbi5idWlsZF9wYXRoKCdTb3VyY2UnLCAnV2Vi S2l0JywgJ2d0aycsICd3ZWJraXQnKV0sCmRpZmYgLS1naXQgYS9Ub29scy9ndGsvZ3RrZG9jLnB5 IGIvVG9vbHMvZ3RrL2d0a2RvYy5weQppbmRleCBmZmFhODMyZWEyMGQzMmQ0ZDA5Y2E4YjU2MTZj NTFiNGY2NmFjMTJlLi40NDk0MjIwZTQ2ZDAxMWYyNGUyNTNkNTY0MzY1ZTcyNTJiYzc5NzczIDEw MDY0NAotLS0gYS9Ub29scy9ndGsvZ3RrZG9jLnB5CisrKyBiL1Rvb2xzL2d0ay9ndGtkb2MucHkK QEAgLTM2Myw2ICszNjMsOCBAQCBjbGFzcyBHVEtEb2Mob2JqZWN0KToKICAgICAgICAgICAgICAg ICAnLS1yZWxhdGl2ZScsCiAgICAgICAgICAgICAgICAgJy0taHRtbC1kaXI9JXMnICUgaHRtbF9k aXJdCiAgICAgICAgIGFyZ3MuZXh0ZW5kKFsnLS1vdGhlci1kaXI9JXMnICUgZXh0cmFfZGlyIGZv ciBleHRyYV9kaXIgaW4gc2VsZi5jcm9zc19yZWZlcmVuY2VfZGVwc10pCisgICAgICAgIGlmIHNl bGYuZGVzdGRpcjoKKyAgICAgICAgICAgIGFyZ3MuZXh0ZW5kKFsnLS1kZXN0LWRpcj0lcycgJSBz ZWxmLmRlc3RkaXJdKQogICAgICAgICBzZWxmLl9ydW5fY29tbWFuZChhcmdzLCBjd2Q9c2VsZi5v dXRwdXRfZGlyKQogCiAK 126200 2012-02-08 17:27:58 -0800 2012-02-08 18:40:52 -0800 Patch bug-78177-20120208232638.patch text/plain 3386 gustavo U3VidmVyc2lvbiBSZXZpc2lvbjogMTA3MTUwCmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggNzNlNWUyMWRiN2E4OTVmODQ5ZGU0NmIzNjIzYTA1ZmQ2 OTkzZTlkOS4uMGY0MmZhYWQ1YWE0YzNkYjE2YTJhZTM5YmJmMzhhNDczMTI5ODM0MyAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDIw IEBACiAyMDEyLTAyLTA4ICBHdXN0YXZvIE5vcm9uaGEgU2lsdmEgIDxnbnNAZ25vbWUub3JnPgog CisgICAgICAgIFtHVEtdIGRvYyByZWJhc2luZyBkb2VzIG5vdCByZXNwZWN0IERFU1RESVIKKyAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTc4MTc3CisKKyAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBHTlVtYWtlZmls ZS5hbTogUGFzcyBERVNURElSIG9uIHRvIGdlbmVyYXRlLWd0a2RvYywgd2hlbgorICAgICAgICBj YWxsaW5nIGl0IGZvciByZWJhc2luZy4KKyAgICAgICAgKiBndGsvZ2VuZXJhdGUtZ3RrZG9jOgor ICAgICAgICAoZ2V0X2NvbW1vbl9vcHRpb25zKTogSGFuZGxlIHRoZSBuZXcgLS1kZXN0ZGlyIG9w dGlvbi4KKyAgICAgICAgKiBndGsvZ3RrZG9jLnB5OgorICAgICAgICAoR1RLRG9jLnJlYmFzZV9p bnN0YWxsZWRfZG9jcyk6IElmIGEgZGVzdGRpciBoYXMgYmVlbiBnaXZlbiwgcGFzcworICAgICAg ICBpdCBvbiB0byBndGtkb2MtcmViYXNlLCBhbmQgcHJlZml4IHRoZSBodG1sZGlyIHdpdGggaXQu CisKKzIwMTItMDItMDggIEd1c3Rhdm8gTm9yb25oYSBTaWx2YSAgPGduc0Bnbm9tZS5vcmc+CisK ICAgICAgICAgVW5yZXZpZXdlZCB0eXBvIGZpeCB0aGF0IG1ha2VzIGRvY3MgYnVpbGQgYWdhaW4g Zm9yIHRoZSBndGsyLWJhc2VkCiAgICAgICAgIGxpYnJhcnkuCiAKZGlmZiAtLWdpdCBhL1Rvb2xz L0dOVW1ha2VmaWxlLmFtIGIvVG9vbHMvR05VbWFrZWZpbGUuYW0KaW5kZXggMjNkODExMmI5YjUw ZTg0MzQwOWM2YzJhYWFhMmMxNThmZGZmNTg2MS4uOTQwZGRhN2I1ZmZjZDU5YThjNTExNzdiYTcy ZDQ3ZTRlYTBiNDRkMiAxMDA2NDQKLS0tIGEvVG9vbHMvR05VbWFrZWZpbGUuYW0KKysrIGIvVG9v bHMvR05VbWFrZWZpbGUuYW0KQEAgLTMyNiw3ICszMjYsNyBAQCBpZiBFTkFCTEVfV0VCS0lUMgog CSAgZmk7IFwKIAlmaQogZW5kaWYKLQkkKHNyY2RpcikvVG9vbHMvZ3RrL2dlbmVyYXRlLWd0a2Rv YyAtLXJlYmFzZQorCSQoc3JjZGlyKS9Ub29scy9ndGsvZ2VuZXJhdGUtZ3RrZG9jIC0tcmViYXNl IC0tZGVzdGRpcj0kJHtERVNURElSfQogCiB1bmluc3RhbGwtbG9jYWw6CiAJQERPQ19NT0RVTEVf VkVSU0lPTj1gY2F0IC4vRG9jdW1lbnRhdGlvbi93ZWJraXRndGsvdmVyc2lvbi54bWxgOyBcCmRp ZmYgLS1naXQgYS9Ub29scy9ndGsvZ2VuZXJhdGUtZ3RrZG9jIGIvVG9vbHMvZ3RrL2dlbmVyYXRl LWd0a2RvYwppbmRleCA5OTZiOTFhMDI0MjVhMjU1MGFkYmRhZDQzNjQ3MjM4NWZiNDFhOGQ3Li4y NjYyYjc1OTJkZGE1NzFiNmQ4OWJmMGM1ZDFhYmY3YzQzYTE3ZTVhIDEwMDc1NQotLS0gYS9Ub29s cy9ndGsvZ2VuZXJhdGUtZ3RrZG9jCisrKyBiL1Rvb2xzL2d0ay9nZW5lcmF0ZS1ndGtkb2MKQEAg LTUwLDEwICs1MCwyNyBAQCBkZWYgZ2V0X2d0a2RvY19tb2R1bGVfcGF0aHMoeHJlZl9kZXBfcGFj a2FnZXMpOgogCiAKIGRlZiBnZXRfY29tbW9uX29wdGlvbnMoKToKKyAgICAjIFdlIHNob3VsZCBj b25zaWRlciB1c2luZyBhbiBhcmd1bWVudHMgcGFyc2luZyBsaWJyYXJ5IGlmCisgICAgIyB3ZSBu ZWVkIG1vcmUgb2YgdGhlc2UgY29tcGxleCBvbmVzLgorICAgIGRlc3RkaXIgPSAnJworICAgIHVz ZV9uZXh0ID0gRmFsc2UKKyAgICBmb3IgYXJnIGluIHN5cy5hcmd2OgorICAgICAgICBpZiB1c2Vf bmV4dDoKKyAgICAgICAgICAgIGRlc3RkaXIgPSBhcmcKKyAgICAgICAgICAgIGJyZWFrCisKKyAg ICAgICAgaWYgYXJnLnN0YXJ0c3dpdGgoJy0tZGVzdGRpcicpOgorICAgICAgICAgICAgaWYgJz0n IGluIGFyZzoKKyAgICAgICAgICAgICAgICBkZXN0ZGlyID0gYXJnLnNwbGl0KCc9JylbMV0KKyAg ICAgICAgICAgICAgICBicmVhaworICAgICAgICAgICAgZWxzZToKKyAgICAgICAgICAgICAgICB1 c2VfbmV4dCA9IFRydWUKKwogICAgIHJldHVybiB7CiAgICAgICAgICdkZWNvcmF0b3InOiAnV0VC S0lUX0FQSScsCiAgICAgICAgICdkZXByZWNhdGlvbl9ndWFyZCc6ICdXRUJLSVRfRElTQUJMRV9E RVBSRUNBVEVEJywKICAgICAgICAgJ2xpYnJhcnlfcGF0aCcgOiBjb21tb24uYnVpbGRfcGF0aCgn LmxpYnMnKSwKKyAgICAgICAgJ2Rlc3RkaXInIDogZGVzdGRpciwKICAgICB9CiAKIGRlZiBnZXRf Y29tbW9uX3hyZWZfZGVwcygpOgpkaWZmIC0tZ2l0IGEvVG9vbHMvZ3RrL2d0a2RvYy5weSBiL1Rv b2xzL2d0ay9ndGtkb2MucHkKaW5kZXggZmZhYTgzMmVhMjBkMzJkNGQwOWNhOGI1NjE2YzUxYjRm NjZhYzEyZS4uZDk2Y2U3NjhlMjFhYmNmNGExZTE1MTU1Yzk4NWU1ZGIyYWViYTkzYyAxMDA2NDQK LS0tIGEvVG9vbHMvZ3RrL2d0a2RvYy5weQorKysgYi9Ub29scy9ndGsvZ3RrZG9jLnB5CkBAIC0z NTYsMTMgKzM1NiwxNSBAQCBjbGFzcyBHVEtEb2Mob2JqZWN0KToKICAgICAgICAgc2VsZi5fcnVu X2NvbW1hbmQoYXJncywgY3dkPXNlbGYub3V0cHV0X2RpciwgaWdub3JlX3dhcm5pbmdzPVRydWUp CiAKICAgICBkZWYgcmViYXNlX2luc3RhbGxlZF9kb2NzKHNlbGYpOgotICAgICAgICBodG1sX2Rp ciA9IG9zLnBhdGguam9pbihzZWxmLnByZWZpeCwgJ3NoYXJlJywgJ2d0ay1kb2MnLCAnaHRtbCcs IHNlbGYubW9kdWxlX25hbWUpCisgICAgICAgIGh0bWxfZGlyID0gb3MucGF0aC5qb2luKHNlbGYu ZGVzdGRpciArIHNlbGYucHJlZml4LCAnc2hhcmUnLCAnZ3RrLWRvYycsICdodG1sJywgc2VsZi5t b2R1bGVfbmFtZSkKICAgICAgICAgaWYgbm90IG9zLnBhdGguaXNkaXIoaHRtbF9kaXIpOgogICAg ICAgICAgICAgcmV0dXJuCiAgICAgICAgIGFyZ3MgPSBbJ2d0a2RvYy1yZWJhc2UnLAogICAgICAg ICAgICAgICAgICctLXJlbGF0aXZlJywKICAgICAgICAgICAgICAgICAnLS1odG1sLWRpcj0lcycg JSBodG1sX2Rpcl0KICAgICAgICAgYXJncy5leHRlbmQoWyctLW90aGVyLWRpcj0lcycgJSBleHRy YV9kaXIgZm9yIGV4dHJhX2RpciBpbiBzZWxmLmNyb3NzX3JlZmVyZW5jZV9kZXBzXSkKKyAgICAg ICAgaWYgc2VsZi5kZXN0ZGlyOgorICAgICAgICAgICAgYXJncy5leHRlbmQoWyctLWRlc3QtZGly PSVzJyAlIHNlbGYuZGVzdGRpcl0pCiAgICAgICAgIHNlbGYuX3J1bl9jb21tYW5kKGFyZ3MsIGN3 ZD1zZWxmLm91dHB1dF9kaXIpCiAKIAo=