plexoos
commented
2 years ago Is it really necessary to remove all mgr/Make*.mk files in order to compile with ROOT6?
Closed perevbnlgov closed 2 years ago
plexoos
commented
2 years ago Is it really necessary to remove all mgr/Make*.mk files in order to compile with ROOT6?
klendathu2k
commented
2 years ago Current version does not work yet for "pams" with non standard Fortran code.
Depending on which codes under pams do not work... this presents a possible show stopper. Could you provide more detail on which codes under pams fails to compile, and what "nonstandard" FORtran code you are refering to?
On 2021-07-16 15:49, perevbnlgov wrote:
"Cons" changes for Root6, merging of DEV Cons + Y.Fisyak + VP. Cons in STAR itself over complicated code. It is very hard to support. Probably in future it is better to move to CMAKE. Current version does not work yet for "pams" with non standard Fortran code.
StarGenerator: Full version. Buf i Cons fixed and now full version compiles successfully.
Some old SAR code, based on Root5 features and not used now is obsolete. Few technical changes for for Root6.
YOU CAN VIEW, COMMENT ON, OR MERGE THIS PULL REQUEST ONLINE AT:
https://github.com/star-bnl/star-sw/pull/59
COMMIT SUMMARY
- Added include "StMessMgr.h"
- Star mgr for Star6/Root6 merge of STAR + Y.Fisyak + VP
FILE CHANGES
- M StRoot/StBTofUtil/StVpdSimConfig.h [1] (1)
- M StRoot/StChain/StMaker.cxx [2] (2)
- M StRoot/StPxlSimMaker/DIGMAPS/digresult.h [3] (2)
- M StRoot/StTofHitMaker/StTofHitMaker.h [4] (24)
- M StRoot/StUtilities/StMultiH1F.h [5] (4)
- M StRoot/St_QA_Maker/StQAMakerBase.cxx [6] (4)
- A StRoot/St_base/St.cxx [7] (92)
- A StRoot/St_base/St.h [8] (48)
- A StRoot/StarGenerator/EvtGen1_06_00/src/EvtGenExternal/EvtPythiaEngine.cpp/EvtPythiaEngine.cpp [9] (828)
- A StRoot/StarGenerator/Pythia8_1_62/include/Event.h/Event.h [10] (582)
- A StRoot/StarGenerator/Pythia8_1_62/include/ProcessContainer.h/ProcessContainer.h [11] (182)
- A StRoot/StarGenerator/Pythia8_1_62/include/Pythia.h/Pythia.h [12] (341)
- A StRoot/StarGenerator/Pythia8_1_86/StarPythia8Decayer.cxx/StarPythia8Decayer.cxx [13] (151)
- M StRoot/macros/Load.C [14] (34)
- A StarVMC/minicern/csmytokn.cxx [15] (44)
- M StarVMC/minicern/jumpxn.c [16] (2)
- M StarVMC/minicern/lnxppcgs/ublow.F [17] (2)
- M StarVMC/minicern/lnxppcgs/ubunch.F [18] (4)
- M StarVMC/minicern/sungs/datimh.F [19] (7)
- D mgr/.pawlogon.kumac [20] (6)
- M mgr/BFCOpt2Html.pl [21] (4)
- A mgr/BuildTables.pl [22] (132)
- D mgr/CERN_LEVEL.sun4x_59 [23] (1)
- M mgr/CleanLibs [24] (11)
- A mgr/ClearBrokenLibs.pl [25] (23)
- M mgr/ConsDefs.pm [26] (1886)
- M mgr/Conscript-standard [27] (13)
- M mgr/ConstructTable.pl [28] (55)
- D mgr/MakeAsp.mk [29] (491)
- D mgr/MakeDep.mk [30] (48)
- D mgr/MakeDirs.mk [31] (67)
- D mgr/MakeDll.mk [32] (735)
- D mgr/MakeEnv.mk [33] (313)
- D mgr/MakeExe.mk [34] (204)
- D mgr/MakeFun.mk [35] (87)
- D mgr/MakeGe3.mk [36] (61)
- D mgr/MakePam.mk [37] (702)
- D mgr/MakeRexe.mk [38] (224)
- D mgr/MakeSl.mk [39] (76)
- D mgr/MakeStaf.mk [40] (180)
- D mgr/Makeloop.mk [41] (500)
- M mgr/ROOT_LEVEL [42] (3)
- M mgr/RootCint.pl [43] (79)
- A mgr/RootCling.pl [44] (429)
- M mgr/cons [45] (45)
- D mgr/cons.1 [46] (2698)
- A mgr/cpu_per_event.csh [47] (11)
- D mgr/embedding/Chain_object.pm [48] (601)
- D mgr/embedding/EmbeddingUtilities.pm [49] (283)
- D mgr/embedding/Embedding_sge_noFTPC.pl [50] (343)
- D mgr/embedding/Process_object.pm [51] (849)
- D mgr/embedding/bfc.pl [52] (240)
- D mgr/embedding/bfc_sge.pl [53] (296)
- A mgr/find.pl [54] (106)
- M mgr/find_dead_link.pl [55] (7)
- M mgr/fiterr.csh [56] (26)
- M mgr/fixrootmk [57] (18)
- D mgr/g2t [58] (74)
- A mgr/gccfilter [59] (451)
- A mgr/good_events.csh [60] (1)
- D mgr/installExe.csh [61] (51)
- D mgr/installRexe.csh [62] (59)
- D mgr/installStaf.csh [63] (57)
- D mgr/lsFile.csh [64] (4)
- M mgr/mv_jobfiles.pl [65] (4)
- D mgr/perltidy.1 [66] (933)
- M mgr/readmail.pl [67] (4)
- D mgr/rootconf.csh [68] (2)
- M mgr/sharedlib_check.pl [69] (2)
- D mgr/sunWS [70] (1)
- M mgr/xtitl [71] (14)
PATCH LINKS:
-- You are receiving this because your review was requested. Reply to this email directly, view it on GitHub [72], or unsubscribe [73].
Links:
[1] https://github.com/star-bnl/star-sw/pull/59/files#diff-c90bbca39864f1c24196b3818c1c9e186df8593728bedd8e4bfea0fa693845ab [2] https://github.com/star-bnl/star-sw/pull/59/files#diff-28079b014413cd8dc5b0d654fbc4d958878885cf922d77c2d1ad4650646a0a91 [3] https://github.com/star-bnl/star-sw/pull/59/files#diff-3e405b24c6da9c3b46eed57c7c7e4fb186ce7a74ff0c9f792265f5a813e677c2 [4] https://github.com/star-bnl/star-sw/pull/59/files#diff-d62d4af5f1b3f5f62bb5b28676f36aaec9575a36b78a7c2f2e1b4cedc1154710 [5] https://github.com/star-bnl/star-sw/pull/59/files#diff-9f7f79eda6d9b59e5c14808e785f0c401c5cc6d90df5719c46439d2b34bb1540 [6] https://github.com/star-bnl/star-sw/pull/59/files#diff-e7fd6970a45037cb56ad9a6d4cb418e0915cb8a138d35a64f732419554e651bb [7] https://github.com/star-bnl/star-sw/pull/59/files#diff-e30f17f3050f2bbd10ba3bc4ec11ac85912cd73d17d733b53705284c26a701e5 [8] https://github.com/star-bnl/star-sw/pull/59/files#diff-57cb611d78268eeddc3e062f526b8620327b500ded0634b4f910320884528933 [9] https://github.com/star-bnl/star-sw/pull/59/files#diff-3a5e1c21a574c824d2fb1bdc3098f8021480ab70ac9b494a143dac3d8e9af30a [10] https://github.com/star-bnl/star-sw/pull/59/files#diff-5e62b30d26d2872407a9263370058c08fb1e9855c6c5a0d328087905fa2c536d [11] https://github.com/star-bnl/star-sw/pull/59/files#diff-dfdb5133a7f15633b750add4b291f3e3598d61863be4c8b631ca47bdab4cc1ac [12] https://github.com/star-bnl/star-sw/pull/59/files#diff-21e2621ccbf5a3ba14c00739251cac7229a9e3445aefcd0647eb575eb1c939e1 [13] https://github.com/star-bnl/star-sw/pull/59/files#diff-2652f55412bd651366de5991104b1704bde30309f0d6087d9b355af79d33e167 [14] https://github.com/star-bnl/star-sw/pull/59/files#diff-d254fa48c6114f6261c944a673428f30e08decc9dff31d803d41593e1c4ab2a3 [15] https://github.com/star-bnl/star-sw/pull/59/files#diff-0d078d701ad99ea111fd09d51046de8226fd33f570b46ea9650a5ded29779c18 [16] https://github.com/star-bnl/star-sw/pull/59/files#diff-2cca1d7ef381755a4dafd3acf2881abf50e43fb011ec4f183af2e7412240b82c [17] https://github.com/star-bnl/star-sw/pull/59/files#diff-45443a989441bf93a6fafbe03ce2ea82d82857e359e8739fb7e222e65cd85e65 [18] https://github.com/star-bnl/star-sw/pull/59/files#diff-8e70c67328ce2692473e57badb7acbb4d103f3632d844765a7b315f2acfd4a93 [19] https://github.com/star-bnl/star-sw/pull/59/files#diff-d0dd902a63e9c235003c921e2e0f432aeeca507740c3e4b76114f34b0c561639 [20] https://github.com/star-bnl/star-sw/pull/59/files#diff-6468a32fe39ab49b9627d17b36613f9bc76ac7f3c6b3add32ed9191af63f1591 [21] https://github.com/star-bnl/star-sw/pull/59/files#diff-7be9490cea5eebcd0e6895c75b0eee52aa3b979ac2c953c5d7dde75a3006d0ac [22] https://github.com/star-bnl/star-sw/pull/59/files#diff-c9cf0c097c92930c5554264815048cbffc5910ca29c460a0b11cb3c94cab5d3e [23] https://github.com/star-bnl/star-sw/pull/59/files#diff-35d79c524634997fb840ac117709be1325886e32054c0cb9e6e127c62ce9a059 [24] https://github.com/star-bnl/star-sw/pull/59/files#diff-c93d35b2ee08497a6dbc3869f0f2956a1a8a370d1840a9b1381af69fd8569be5 [25] https://github.com/star-bnl/star-sw/pull/59/files#diff-517b601ad7223ac0d8ed42f8cd204bbe473c2f4a0f175f18c584220c0d2fe64f [26] https://github.com/star-bnl/star-sw/pull/59/files#diff-81b0c2ea1484965ccf19627026e038ceef71d00acb02743bc63626c6772de7c9 [27] https://github.com/star-bnl/star-sw/pull/59/files#diff-1a8e1c2547fe827a28777b087e85ff549033fdd9208cdfe0e53ed5607d61e0d1 [28] https://github.com/star-bnl/star-sw/pull/59/files#diff-356c6255be2499ab5866e76e0848605eaf525703aeeecf88dc3405bca54f0950 [29] https://github.com/star-bnl/star-sw/pull/59/files#diff-9a325ea399e183bb3396feb460de373929b4ba5410b59826772f479e3c274e9f [30] https://github.com/star-bnl/star-sw/pull/59/files#diff-198c63e5370bcb84d54d9311e33313b04aa59676b2f27c85899387b7e74fe9a9 [31] https://github.com/star-bnl/star-sw/pull/59/files#diff-cebdec1ff10c6a0f36997e74f0bfee7121f9405c7a143068ac01dd3dff449abf [32] https://github.com/star-bnl/star-sw/pull/59/files#diff-33cd24be2741c1514fc316e468383a867f7ade053020960f70f4654e1461effc [33] https://github.com/star-bnl/star-sw/pull/59/files#diff-e11aeef6f5bcb07d63ebe9bd2a2addfc0f4388d2c9b5b30663447203de86f9dd [34] https://github.com/star-bnl/star-sw/pull/59/files#diff-0bc096beecf290c911d37271a64c24aeb944935b21cf11b5a8d2acf2b05faccb [35] https://github.com/star-bnl/star-sw/pull/59/files#diff-06dbd0a7102a0a87ae92a4d9ad26e968801f895387283740e1f61cf2b4946135 [36] https://github.com/star-bnl/star-sw/pull/59/files#diff-043c210338a7d3ab0e376d7d90a58e67c495507a814a581ee1d04d7d69fe3466 [37] https://github.com/star-bnl/star-sw/pull/59/files#diff-219a3fb6e4c6932a9e4799e0e5751bb4ce0e40cf669c3a2c9d3d19d4ac4990e4 [38] https://github.com/star-bnl/star-sw/pull/59/files#diff-0405f334b26bc4e485bcb66375a5c78977c881873f6390601f25f19272f031d3 [39] https://github.com/star-bnl/star-sw/pull/59/files#diff-62ae59d82562925cb8a06b25fef8870971c9391ab6b4824cd7a472b0655013d6 [40] https://github.com/star-bnl/star-sw/pull/59/files#diff-a58efec4fb95d9fe88d53e13189ed2c85ab9ffbe1c53c4f30bdb58f720f3bcc4 [41] https://github.com/star-bnl/star-sw/pull/59/files#diff-2d0126aa924cef8f7c755dd9b1245408cf5325327409d19387ac7c8d00fa96bc [42] https://github.com/star-bnl/star-sw/pull/59/files#diff-9cc6430148086095700c914626218d4f8ee8b3781242b0f2187087e5a42c198e [43] https://github.com/star-bnl/star-sw/pull/59/files#diff-e59be536d34ff9de20d36de6039c053bd9e3763e8640cf90a14ddc3e7bc7ff40 [44] https://github.com/star-bnl/star-sw/pull/59/files#diff-5a1145fdb1a7435f2b6678db876042e3855f9fe6ec28a118096dee33dad122b0 [45] https://github.com/star-bnl/star-sw/pull/59/files#diff-571a90f85a4d4fa610f80c7f78374aa15137e22d6cc8db8a60a2de898145b2f8 [46] https://github.com/star-bnl/star-sw/pull/59/files#diff-840c3bfef78698345097f6778899c7cc3165565fd591da20cf59594d050a1ab2 [47] https://github.com/star-bnl/star-sw/pull/59/files#diff-803527c54b3917a6bdc4aea6e00234f6a181d7dbc9bde4f4bd6777ff5fea2598 [48] https://github.com/star-bnl/star-sw/pull/59/files#diff-1e65af3dfec2e6d0e50b32d42626c962c9ea0bef4f305fb7433fa6ec08e6578a [49] https://github.com/star-bnl/star-sw/pull/59/files#diff-266473d21940a7dbb1ef645b011976c907770a1ac5879cc307067d555e3d7ccb [50] https://github.com/star-bnl/star-sw/pull/59/files#diff-6510199db2545e2152ac0811abbd5a78a451da01a2d3360e734a41353136e782 [51] https://github.com/star-bnl/star-sw/pull/59/files#diff-0dc28f3ce60f9a26c4115f7f75dd5201cf0505e208ac63f10fb4219c3646c303 [52] https://github.com/star-bnl/star-sw/pull/59/files#diff-f869ea35e10a69efe3ed2c275b2810a6f3eaeea88d36b9961c555bab1483a51a [53] https://github.com/star-bnl/star-sw/pull/59/files#diff-3afccd06271db728ce457a20ae6b61efd915bae6cda3cd01be3ed3dc65212077 [54] https://github.com/star-bnl/star-sw/pull/59/files#diff-012d4813687ec98f2b7dee14ee6692d55bdd75e19098e11734ffb170fd814468 [55] https://github.com/star-bnl/star-sw/pull/59/files#diff-37a8a8687df9daa9a3fd007f7b7ec2c58eb76c84b9ef07354823ecd9c68355d8 [56] https://github.com/star-bnl/star-sw/pull/59/files#diff-74c9dade1f6c7b3921d66f4c1bbe6bf07c1e9840106081e64384d9581198fb20 [57] https://github.com/star-bnl/star-sw/pull/59/files#diff-4759d4c75fae0013d3a1799d07496826305460b36e8296321ca4320be4b7e2d0 [58] https://github.com/star-bnl/star-sw/pull/59/files#diff-dab55d6ebee246fc1ac42b3dfee89e42945958b12bcf193fe8c6108ea4f22f71 [59] https://github.com/star-bnl/star-sw/pull/59/files#diff-ff44499562f28068088275c4fc5663ae1a255f47fa170aede8196aae8e4405a0 [60] https://github.com/star-bnl/star-sw/pull/59/files#diff-162bb9ecdc10cb0dcb23ceaacbedc27c2e030e1e082a7a60cae717ce0441ba36 [61] https://github.com/star-bnl/star-sw/pull/59/files#diff-3116029360d11713cda036ccbbb2a278c384c873b9e7e58dcd7850100175e379 [62] https://github.com/star-bnl/star-sw/pull/59/files#diff-23380dab9fbcbef6e7b47dc14b797fb06a262f08ca2be48f7a7033766df1cec0 [63] https://github.com/star-bnl/star-sw/pull/59/files#diff-265f3137f0bb6a6d3665b09a04110fe94c1b17122e75c3d302055fc5975518cf [64] https://github.com/star-bnl/star-sw/pull/59/files#diff-bcde59ed02fe938d5224d5e2b23ca4eed213f0ac1211095cfdb9084c1a225147 [65] https://github.com/star-bnl/star-sw/pull/59/files#diff-3c60e2a5f904bd1d8440365d737b2ba6ddd44580d91698f12d4e43e6781b4160 [66] https://github.com/star-bnl/star-sw/pull/59/files#diff-e56916b7859aa0dc67a029efd29fa824a39e9381cf6c5ed0148bc167adb0733f [67] https://github.com/star-bnl/star-sw/pull/59/files#diff-3e3be7bbd46f9a7b97f5a4f212f166f98f092e665cfa3c8c684fdc56c9ec9511 [68] https://github.com/star-bnl/star-sw/pull/59/files#diff-95766ea2c24ac7244e978798d5125b76e3fade93750f82f61ab447281516620c [69] https://github.com/star-bnl/star-sw/pull/59/files#diff-850c3b8e549644a1983fa193c0955e881bb3eb32cec9dce5c97de2edced43b75 [70] https://github.com/star-bnl/star-sw/pull/59/files#diff-3538cae062fc3212204dfacf96aed1601109886f52d14fd8199871b0034fe6e9 [71] https://github.com/star-bnl/star-sw/pull/59/files#diff-b5a46e48a3419376e78d698cfa7cd9dbcda09ce6359091083fd8336cce4da5f3 [72] https://github.com/star-bnl/star-sw/pull/59 [73] https://github.com/notifications/unsubscribe-auth/ANL4LVH6C73FHZTNR6UDZOTTYCEMVANCNFSM5AQFGM3A
perevbnlgov
commented
2 years ago All these mgr/Make* were used when STAR was compiled by gmake. Then Yuri change it to Cons. It was about 15 years before Victor
On 2021-07-16 16:28, Dmitri Smirnov wrote:
Is it really necessary to remove all mgr/Make*.mk files in order to compile with ROOT6?
-- You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub [1], or unsubscribe [2].
Links:
[1] https://github.com/star-bnl/star-sw/pull/59#issuecomment-881699990 [2] https://github.com/notifications/unsubscribe-auth/ANQUL7PCZPBKQWXVCKHQDS3TYCI5BANCNFSM5AQFGM3A
klendathu2k
commented
2 years ago Hi Victor,
I think the objection is the relevance to ROOT6. Removing the makefiles should be an independent pull request. In fact, there are numerous instances where irrelevant changes are made.
https://github.com/star-bnl/star-sw/pull/59/files/042bae36eb1af4423e02363aa226989add59e71c
Jason
p.s. Not quite true that the makefiles haven't been used for 15 years.
The "make" command
in starsim was disabled ~5 years ago. It still relied on some of these
makefiles.
On 2021-07-17 12:35, perevbnlgov wrote:
All these mgr/Make* were used when STAR was compiled by gmake. Then Yuri change it to Cons. It was about 15 years before Victor
On 2021-07-16 16:28, Dmitri Smirnov wrote:
Is it really necessary to remove all mgr/Make*.mk files in order to compile with ROOT6?
-- You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub [1], or unsubscribe [2].
Links:
[1] https://github.com/star-bnl/star-sw/pull/59#issuecomment-881699990 [2]
https://github.com/notifications/unsubscribe-auth/ANQUL7PCZPBKQWXVCKHQDS3TYCI5BANCNFSM5AQFGM3A
You are receiving this because your review was requested. Reply to this email directly, view it on GitHub [1], or unsubscribe [2].
Links:
[1] https://github.com/star-bnl/star-sw/pull/59#issuecomment-881923812 [2] https://github.com/notifications/unsubscribe-auth/ANL4LVBJWTHZVPXO53N3XRDTYGWNDANCNFSM5AQFGM3A
perevbnlgov
commented
2 years ago Hi Jason,
Depending on which codes under pams do not work... this presents a possible show stopper. As we agree I must save my modifications to avoid repetition of my previous accidentally deletion of files. This pams/sim does not work because of written in VAX extended or PGI fortran. Probably my changes oe merging to Yuri's cons affects it. Anyway I think soon it will be fixed. Also it is in Star6 branch only. You can see the errors:
gfortran -DCERNLIB_TYPE -DCERNLIB_DOUBLE -DCERNLIB_NOQUAD -DCERNLIB_LINUX -Dsl73_x8664_gcc485 -m64 -fd-lines-as-code -std=legacy -fno-second-underscore -w -fno-automatic -Wall -W -Wsurprising -fPIC -ffixed-line-length-132 -I.sl73_x8664_gcc485/include -Iasps/Simulation/geant321/include -I. -I/cern64/pro/include -c .sl73_x8664_gcc485/obj/pams/sim/g2r/g2t_rch.F -o .sl73_x8664_gcc485/obj/pams/sim/g2r/g2t_rch.o; .sl73_x8664_gcc485/include/table_header.inc:7.7: Included at .sl73_x8664_gcc485/include/PAM.inc:18: Included at .sl73_x8664_gcc485/include/g2t_rch.inc:14: Included at .sl73_x8664_gcc485/obj/pams/sim/g2r/g2t_rch.F:5:
STRUCTURE /table_head_st/
1Error: Unclassifiable statement at (1) .sl73_x8664_gcc485/include/table_header.inc:21.10: Included at .sl73_x8664_gcc485/include/PAM.inc:18: Included at .sl73_x8664_gcc485/include/g2t_rch.inc:14: Included at .sl73_x8664_gcc485/obj/pams/sim/g2r/g2t_rch.F:5:
END STRUCTURE ! table_head_stVictor
On 2021-07-17 08:24, klendathu2k wrote:
Current version does not work yet for "pams" with non standard Fortran code.
Depending on which codes under pams do not work... this presents a possible show stopper. Could you provide more detail on which codes under pams fails to compile, and what "nonstandard" FORtran code you are refering
to?
On 2021-07-16 15:49, perevbnlgov wrote:
perevbnlgov
commented
2 years ago It is related to Cling which is much less stable than Cint. Moving variables to private anyway is more safe.
The same reason of changing ClassDef(..,0) instead of 1. In my previous, deleted version, all ClassDef were changed to 0. Now I changed only when Cling has a problems.
Amyway 0 in ClassDef means no I/O for maker. But our makers are not supposed to have it Victor
On 2021-07-17 08:28, klendathu2k wrote:
@klendathu2k commented on this pull request.
In StRoot/StTofHitMaker/StTofHitMaker.h [1]:
@@ -55,6 +43,16 @@ class StTofHitMaker:public StRTSBaseMaker Int_t UnpackTofRawData(); void fillTofDataCollection(); void fillStEvent(); //! ship collection to StEvent
- /// TOF Raw hits info. struct
Cleanup / TOF hit structure moved to private.
-- You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub [2], or unsubscribe [3].
Links:
[1] https://github.com/star-bnl/star-sw/pull/59#discussion_r671691306 [2] https://github.com/star-bnl/star-sw/pull/59#pullrequestreview-708938139 [3] https://github.com/notifications/unsubscribe-auth/ANQUL7PMHOECXR3ICCYDKTDTYFZORANCNFSM5AQFGM3A
perevbnlgov
commented
2 years ago Hi Jason,
I recommend anything that has to do with code cleanup be separated its own pull request, and that we do code cleanup first. e.g. removing makefiles that haven't been used for decades, etc... In mgr/ there is no distinct separation between system code and user one. In result names of makers exists in all the perl files. To make this mess more clear I removed all the redundant files which are not used anymore. When "cons" will work again, I can put all these files back, if you want:-)
There are things being added (log file parsers, a c++ filter script) that are not needed. These should be removed from the PR. There are also some changes to codes which probably do not need not be changed for ROOT6. These should be omitted. Sorry, I do not understand what you are talking about?
I don't know why code from StarGenerator is being included into the star-sw repository, and why it is necessary to reshape the directory structure under the event generators. I assume the issue is that Victor did not checkout star-mcgen during this work. But if not, then we need to resolve the problem with cons rather than hack something that works. StarGenerator code was committed unto Git only partially. So now I commited it completely, as it was in STARDEV. If it was your decision to drop some part of it, I have no objections. What about "star-mcgen:? I can not found it in STARDEV. Is it something new, which I missed by mistake?
Changes to minicern look like 64bit support, rather than ROOT6. Again, we should be making a PR that is specific to the task at hand. Support for ROOT6. If there are other modifications which need to be made, they should be separated out. You right, it is not related to Root6. But if you have many compilation errors it is not easy to select them in different kinds. It is rather easy to do in CVS, which I already did , but in Git it is very complicated.
The class "St"... really needs a better name. Suggest StCintClingHelper. Using if this name is St::call(...). I was trying to do as short as possible. If to do it directly by gROOT->ProcessLine it will be extremely long line and even StCintClingHelper::call(...) is too long
mgr/.pawlogon.kumac was removed. Not a ROOT6 issue... and this file may be copied into asps/ during the build process. So don't get rid of this.
I thought that this file is a working file committed somebody by mistake. If it is not the case, it is easy to put it back
Victor
In StRoot/St_base/St.h [2]:
@@ -0,0 +1,48 @@ +/***
I would really like to see a more descriptive class name than just "St". (It also technically violates our class naming convention which requires a capital letter after "St".) Propose StCintClingHelper.
perevbnlgov
commented
2 years ago File mgr/RootCling.pl I got from Yuri. As Yuri explained me, Root6 can work in two ways.
Automatic loading is not stable yet. In a future may be automatic loading will be selected Victor
On 2021-07-19 01:46, Dmitri Smirnov wrote:
@plexoos commented on this pull request.
In mgr/RootCling.pl [1]:
@@ -0,0 +1,429 @@ +#!/usr/bin/env perl +# +# Script to run rootcling $Lib $libDep $RootMap Cint_cxx list of h-files
This file mgr/RootCling.pl is new. Where is it used? I see that mgr/RootCint.pl is used by mgr/Conscript-standard but no reference to mgr/RootCling.pl
-- You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub [2], or unsubscribe [3].
Links:
[1] https://github.com/star-bnl/star-sw/pull/59#discussion_r672002253 [2] https://github.com/star-bnl/star-sw/pull/59#pullrequestreview-709169164 [3] https://github.com/notifications/unsubscribe-auth/ANQUL7MGQFDHEIWCWX5CZB3TYO32FANCNFSM5AQFGM3A
plexoos
commented
2 years ago So, you are confirming that mgr/RootCling.pl is not needed right now and the existing mgr/RootCint.pl is enough to move forward. OK.
As noted earlier by Jason and myself, there are many changes in this pull request which are not required to switch our libraries to ROOT6. Can you select only and only those changes that absolutely must be included for our bfc.C test jobs to work with ROOT6? Can you create a new PR with only such changes based on what we already have on the main branch?
starsdong
commented
2 years ago Hi, Victor and All, Shall we have a discussion tomorrow at the S&C management meeting about how to review these changes and integrate them into STAR? There are many files changes and there are a bunch of reviewers on the list. It seems to me it would be best to have a small group of dedicated reviewers to go through them. I would propose we discuss this tomorrow. Certainly all technical comments feel free to continue the discussions.
perevbnlgov
commented
2 years ago So, you are confirming that mgr/RootCling.pl is not needed right now and the existing mgr/RootCint.pl is enough to move forward. OK. It is not so evident. Very probably this RootCling will be needed in future. To keep it separately somewhere is very inconvenient. Victor
On 2021-07-20 10:48, Dmitri Smirnov wrote:
So, you are confirming that mgr/RootCling.pl is not needed right now and the existing mgr/RootCint.pl is enough to move forward. OK.
As noted earlier by Jason and myself, there are many changes in this pull request which are not required to switch our libraries to ROOT6. Can you select only and only those changes that absolutely must be included for our bfc.C test jobs to work with ROOT6? Can you create a new PR with only such changes based on what we already have on the main branch?
-- You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub [1], or unsubscribe [2].
Links:
[1] https://github.com/star-bnl/star-sw/pull/59#issuecomment-883454714 [2] https://github.com/notifications/unsubscribe-auth/ANQUL7IKFWVETIUCD7SLDC3TYWEENANCNFSM5AQFGM3A
plexoos
commented
2 years ago Victor, thank you for reducing the number of changes submitted in this pull request. I assume you noticed that one of them broke the ROOT5 build.
Can you illustrate with an example how you propose to use the new St class in our existing macros?
perevbnlgov
commented
2 years ago Hi guys,
pull request. I assume you noticed that one of them broke the ROOT5 build. The only thing which I did, I cleared my "Forked" and now I a ready to make commiit. My forked repository in branch Statr6 is cleared for the level of master. I do not understand how I can broke anything, especially Root5 which I did not touch at all Victor
On 2021-08-20 10:59, Dmitri Smirnov wrote:
Victor, thank you for reducing the number of changes submitted in this pull request. I assume you noticed that one of them broke the ROOT5 build.
Can you illustrate with an example how you propose to use the new St class in our existing macros?
-- You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub [1], or unsubscribe [2]. Triage notifications on the go with GitHub Mobile for iOS [3] or Android [4].
Links:
[1] https://github.com/star-bnl/star-sw/pull/59#issuecomment-902755202 [2] https://github.com/notifications/unsubscribe-auth/ANQUL7KDG6AC3WSGTRWKPWTT5ZUULANCNFSM5AQFGM3A [3] https://urldefense.com/v3/__https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675__;!!P4SdNyxKAPE!XgHN9xN8c1K2ic2dO51xRUHPJUVmqdyoFDqb9PSX4vEN4sNqegomNt1oWV3rIw$ [4] https://urldefense.com/v3/__https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email__;!!P4SdNyxKAPE!XgHN9xN8c1K2ic2dO51xRUHPJUVmqdyoFDqb9PSX4vEN4sNqegomNt1ki9PJEg$
veprbl
commented
2 years ago I do not understand how I can broke anything, especially Root5 which I did not touch at all
Please see in the end of the build log https://github.com/star-bnl/star-sw/pull/59/checks?check_run_id=3373872128
perevbnlgov
commented
2 years ago Please see in the end of the build log https://github.com/star-bnl/star-sw/pull/59/checks?check_run_id=3373872128 I do not see any my modifications there. Anyway, I did not do any modifications in the main repository weeks ago Victor
On 2021-08-20 12:08, Dmitry Kalinkin wrote:
I do not understand how I can broke anything, especially Root5 which I did not touch at all
Please see in the end of the build log https://github.com/star-bnl/star-sw/pull/59/checks?check_run_id=3373872128
-- You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub [1], or unsubscribe [2]. Triage notifications on the go with GitHub Mobile for iOS [3] or Android [4].
Links:
[1] https://github.com/star-bnl/star-sw/pull/59#issuecomment-902800025 [2] https://github.com/notifications/unsubscribe-auth/ANQUL7OQ5T64ZB45XZF6S3DT5Z4XFANCNFSM5AQFGM3A [3] https://urldefense.com/v3/__https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675__;!!P4SdNyxKAPE!TRLaav1A-KfI-a4oF7Kys44EcWesiCTGEfC3r9miIrrdSDdQMBhANjcL6V0nug$ [4] https://urldefense.com/v3/__https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email__;!!P4SdNyxKAPE!TRLaav1A-KfI-a4oF7Kys44EcWesiCTGEfC3r9miIrrdSDdQMBhANjfvYFZY_A$
veprbl
commented
2 years ago I do not see any my modifications there.
It shows an error with St.h that you are looking to contribute:
#20 2361.5 virtual Int_t Write(const char *name=0, Int_t option=0, Int_t bufsize=0);
14826
#20 2361.5 ^
14827
#20 2905.0 In file included from .sl88_gcc789/OBJ/StRoot/St_base/St.cxx:11:0:
14828
#20 2905.0 .sl88_gcc789/OBJ/StRoot/St_base/St.h:17:24: fatal error: RtypesCore.h: No such file or directory
14829
#20 2905.0 #include "RtypesCore.h"
14830
#20 2905.0 ^
14831
#20 2905.0 compilation terminated.
14832
#20 2905.1 cons: *** [.sl88_gcc789/OBJ/StRoot/St_base/St.o] Error 1
14833
#20 2905.1 cons: errors constructing .sl88_gcc789/OBJ/StRoot/St_base/St.o
14834
#20 ERROR: process "/bin/bash -c source /etc/profile && cd /star-sw && cons +StarVMC/Geometry && cons %OnlTools %StRoot/StShadowMaker %StRoot/StHbtMaker && find /star-sw/.$STAR_HOST_SYS -name *.o -exec rm '{}' \\;" did not complete successfully: exit code: 2Few weeks ago I made a pool request. It was not accepted. Then, I cleared up my Forked, to do my fixed commit again. If somebody accept y old forbidden commit I have no info about it
On 2021-08-20 12:54, Dmitry Kalinkin wrote:
@veprbl commented on this pull request.
In StRoot/St_base/St.h [1]:
@@ -0,0 +1,48 @@ +/***
Can you illustrate with an example how you propose to use the new St class in our existing macros?
Just like @plexoos [2] I am puzzled by the use case of this class. From the commit message where it says
In result your macro does not work if you have any problem in subbmacro, which is not even called.
it can be inferred that we have some malformed macros somewhere. Makes me wonder if we should look into fixing those instead. In any case it's really hard to review a piece of code without seeing an example of its use.
-- You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub [1], or unsubscribe [3]. Triage notifications on the go with GitHub Mobile for iOS [4] or Android [5].
Links:
[1] https://github.com/star-bnl/star-sw/pull/59#discussion_r693088845 [2] https://github.com/plexoos [3] https://github.com/notifications/unsubscribe-auth/ANQUL7KG3PJF63RO6LYCRCDT52CDNANCNFSM5AQFGM3A [4] https://urldefense.com/v3/__https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675__;!!P4SdNyxKAPE!WafXSx5EUWALOiitsaLJzDiOjBN8Enl3Ab6OhbSbxn2Rlu2jRudeZkmjdphKPg$ [5] https://urldefense.com/v3/__https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email__;!!P4SdNyxKAPE!WafXSx5EUWALOiitsaLJzDiOjBN8Enl3Ab6OhbSbxn2Rlu2jRudeZkkVNs6tfA$
plexoos
commented
2 years ago Few weeks ago I made a pool request. It was not accepted. Then, I cleared up my Forked, to do my fixed commit again. If somebody accept y old forbidden commit I have no info about it
Pull requests created on GitHub are strongly tied to a branch, actually two branches, as you pull from one branch to another one.
This pull request (#59) automatically monitors the status of the Star6 branch in your perevbnlgov/star-sw forked repo on GitHub. When you modify that Star6 branch (by pushing from your locally cloned repo on your machine), it is immediately reflected in this PR and all of us involved receive notifications. In addition, your activity triggered an automatic build that we have set up on GitHub. That is why we know that your changes introduced a problem with ROOT5 build. @veprbl has kindly pointed to the specific problem. I hope it is clear that we are not going to accept changes, i.e. merge them into the main branch, until the build tests go through without errors. In fact, it is the whole point of such checks.
perevbnlgov
commented
2 years ago It is hard to understand what is going inside of GitHub. What I did. Because it were complains to my Star6 comments, I found the way to clear up my Forked. The I add my modifications again with more detailed comments. But only the few corrects were found. The only explanations I found that most Star6 corrects to the main repository accepted. Probably this GitHub logic is too complicated and I decided to PP all the rest of corrects. Victor
On 2021-08-20 17:06, Dmitri Smirnov wrote:
Few weeks ago I made a pool request. It was not accepted. Then, I cleared up my Forked, to do my fixed commit again. If somebody accept y old forbidden commit I have no info about it
Pull requests created on GitHub are strongly tied to a branch, actually two branches, as you pull from one branch to another one.
This pull request (#59 [1]) automatically monitors the status of the Star6 branch in your perevbnlgov/star-sw forked repo on GitHub. When you modify that Star6 branch (by pushing from your locally cloned repo on your machine), it is immediately reflected in this PR and all of us involved receive notifications. In addition, your activity triggered an automatic build that we have set up on GitHub. That is why we know that your changes introduced a problem with ROOT5 build. @veprbl [2] has kindly pointed to the specific problem. I hope it is clear that we are not going to accept changes, i.e. merge them into the main branch, until the build tests go through without errors. In fact, it is the whole point of such checks.
-- You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub [3], or unsubscribe [4]. Triage notifications on the go with GitHub Mobile for iOS [5] or Android [6].
Links:
[1] https://github.com/star-bnl/star-sw/pull/59 [2] https://github.com/veprbl [3] https://github.com/star-bnl/star-sw/pull/59#issuecomment-902957338 [4] https://github.com/notifications/unsubscribe-auth/ANQUL7PAYSHYYPXSMICQMNDT527VNANCNFSM5AQFGM3A [5] https://urldefense.com/v3/__https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675__;!!P4SdNyxKAPE!Wiut0dKyC1E4h3zB5Rtmub8c_vZGF9sLrRXd1UXiZl_few9fJ3EoLNeDgsyx5g$ [6] https://urldefense.com/v3/__https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email__;!!P4SdNyxKAPE!Wiut0dKyC1E4h3zB5Rtmub8c_vZGF9sLrRXd1UXiZl_few9fJ3EoLNcxAjBHRg$
veprbl
commented
2 years ago It is hard to understand what is going inside of GitHub. What I did. Because it were complains to my Star6 comments, I found the way to clear up my Forked. The I add my modifications again with more detailed comments. But only the few corrects were found. The only explanations I found that most Star6 corrects to the main repository accepted. Probably this GitHub logic is too complicated and I decided to PP all the rest of corrects. Victor
Let me check if I understand you correctly. You say that after you've submitted the initial version of your Pull Request (PR), there were complaints about it containing unnecessary, irrelevant and breaking changes and there were also requests for explanations about some other changes in that PR. To address that you've recommitted some subset of changes to your local Star6 branch and pushed it to the Star6 branch of your fork at https://github.com/perevbnlgov/star-sw. (I believe we all read your commit message at https://github.com/star-bnl/star-sw/pull/59/commits/05f3ea735fbfb48a749d89d03505f352e12416c3, and I appreciate the extra details, but it appears that there are still questions left about it.) Now your concern is that there were more changes that you intended to propose, but they are still missing in the commits that you've prepared. Does that sound correct?
I would like to suggest to concentrate on the existing changes for now. Could you please review the your changes as we see them proposed in your PR at https://github.com/star-bnl/star-sw/pull/59/files , there are at least 6 independent changes, that would ideally submitted as 6 different PR's (from 6 different branches). Putting all changes in the same PR requires addressing a lot more comments at the same time and increases the complexity of communication, thus I would not advise adding additional modifications except for those intended to address the existing review comments (for those see the threads in https://github.com/star-bnl/star-sw/pull/59). The ultimate goal in this process is to address the review comments by providing explanations to the reviewers and by adjusting the code changes until they satisfy the reviewers. After that is achieved and reviewers are satisfied, the pull request gets merged to the main common repository.
perevbnlgov
commented
2 years ago Hi Dmitri I checked star-bnl/star-sw and compare it with my local star-sw. There is no difference. I think I do not need more efforts about it. Now I am working with OnlineTools. Victor
plexoos
commented
2 years ago Now I am working with OnlineTools.
Hi Victor,
In order to avoid duplicate work please have a look at this pull request #101 and issues #100 and #102
That PR is linked to the pr/ci_build_onltools branch in star-bnl/star-sw and contains all proposed fixes to OnlTools/
The current status is that OnlTools/ can be compiled with both ROOT5 and ROOT6 in our CI jobs.
It would be great if you could review all changes in #101. I also received a private email from Jeff @jml985 promising to review the changes as well.
When we merge #101 the saga with uncompileable packages in our repository will be over. All packages can be compiled against both ROOT5 and ROOT6. (Ok, except the wacky StRoot/ShadowMaker which is expected)
"Cons" changes for Root6, merging of DEV Cons + Y.Fisyak + VP. Cons in STAR itself over complicated code. It is very hard to support. Probably in future it is better to move to CMAKE. Current version does not work yet for "pams" with non standard Fortran code.
StarGenerator: Full version. Buf i Cons fixed and now full version compiles successfully.
Some old SAR code, based on Root5 features and not used now is obsolete. Few technical changes for for Root6.