GEODE-5727 is closed and merged to release/1.7.0 Thank you Jinmei On Thu, Sep 13, 2018 at 9:47 AM Nabarun Nag <n...@apache.org> wrote:
> Thank you Patrick. Please do send a notification email once this is > reverted in the release/1.7.0 branch. > Thank you Jinmei for putting the fix for GEODE-5727 into release/1.7.0. > However the GEODE ticket is still in open state. Can it be closed? > > Regards > Nabarun Nag > > > On Thu, Sep 13, 2018 at 9:21 AM Patrick Rhomberg <prhomb...@pivotal.io> > wrote: > >> Sounds like we have a plan! I'll take it upon myself to do the revert. >> >> On Thu, Sep 13, 2018 at 7:24 AM, Sai Boorlagadda < >> sai.boorlaga...@gmail.com> >> wrote: >> >> > +1 to revert and fix on develop >> > >> > On Wed, Sep 12, 2018, 4:43 PM Nabarun Nag <n...@apache.org> wrote: >> > >> > > Reverting them on release/1.7.0 will bring it to the previous status >> quo, >> > > how all previous releases were done. I don't think anyone will build >> > > release/1.7.0 repeatedly, hence there is no advantage of making build >> > > process faster for that branch. >> > > Whereas on develop a more appropriate solution can be incorporated >> after >> > > discussions. >> > > >> > > Is it acceptable? >> > > >> > > Regards >> > > Nabarun Nag >> > > >> > > On Wed, Sep 12, 2018 at 4:37 PM Patrick Rhomberg < >> prhomb...@pivotal.io> >> > > wrote: >> > > >> > > > Okay. So that information is definitely coming from the >> > > > GemFireVersion.properties file, which explains this issue. Either >> > > > reverting the previous GEODE-5600 changes or resolving merge >> conflicts >> > > from >> > > > PR 2457 would address this issue. >> > > > >> > > > My concern remains about the .buildinfo file, however. Is the >> > > .buildinfo >> > > > redundant at this point and should be? Should it always contain the >> > > > necessary information, with the GemFireVersion.properties file >> > acquiring >> > > > the source information from .buildinfo rather than fetching it again >> > > > itself? Is .buildinfo a convention in distributions with which I am >> > just >> > > > myself unfamiliar? >> > > > >> > > > The path we take here is fundamentally linked to how we want to >> > approach >> > > > GEODE-5600, and with PR 2457 currently open, we could choose any of >> > these >> > > > routes to go. >> > > > >> > > > On Wed, Sep 12, 2018 at 4:13 PM, Nabarun Nag <n...@apache.org> >> wrote: >> > > > >> > > > > @patrick >> > > > > if you build geode release branch 1.7.0 "./gradlew clean build >> > > > > -Dskip.tests=true -xdocs -xjavadoc" and start gfsh from >> > > > > geode-assembly/build/install/apache-geode/bin/gfsh >> > > > > And then type `version --full` you get this >> > > > > >> > > > > gfsh>version --full >> > > > > Build-Date: 2018-09-12 16:07:03 -0700 >> > > > > Build-Id: nnag 0 >> > > > > Build-Java-Version: 1.8.0_181 >> > > > > Build-Platform: Mac OS X 10.13.6 x86_64 >> > > > > Product-Name: Apache Geode >> > > > > Product-Version: 1.7.0 >> > > > > Source-Date: 2018-09-12 16:07:03 -0700 >> > > > > *Source-Repository: unknown* >> > > > > *Source-Revision: unknown* >> > > > > Native version: native code unavailable >> > > > > Running on: /10.118.19.23, 8 cpu(s), x86_64 Mac OS X 10.13.6 >> > > > > >> > > > > As you can notice that Source-Repository and Source-Revision is >> > > missing. >> > > > It >> > > > > should contain the info from the buildinfo file present in >> > > > > geode-assemble/.buildinfo file. It contains the following >> > > > > >> > > > > # >> > > > > #Wed Sep 12 16:07:56 PDT 2018 >> > > > > Source-Date=2018-09-11 15\:56\:48 -0700 >> > > > > Source-Revision=c637193aa61abdfd236ae36b6d9a228fc1e84bcd >> > > > > Source-Repository=release/1.7.0 >> > > > > >> > > > > Hope this helps >> > > > > >> > > > > Regards >> > > > > Nabarun Nag >> > > > > >> > > > > On Wed, Sep 12, 2018 at 3:51 PM Patrick Rhomberg < >> > prhomb...@pivotal.io >> > > > >> > > > > wrote: >> > > > > >> > > > > > I'm happy to work on those reverts, although if Anthony could >> > > elaborate >> > > > > on >> > > > > > where exactly the version information was missing, that assuage >> > some >> > > of >> > > > > my >> > > > > > own worries as to whether it's the right approach. It's still >> not >> > > > clear >> > > > > to >> > > > > > me where .buildinfo is intended to be consumed. >> > > > > > >> > > > > > On Wed, Sep 12, 2018 at 3:08 PM, Nabarun Nag <n...@apache.org> >> > > wrote: >> > > > > > >> > > > > > > Yes Alexander, we are still waiting on the build info reverts >> > from >> > > > > > Patrick, >> > > > > > > so, I think that this can be put into release/1.7.0. >> > > > > > > >> > > > > > > Sure Jinmei, you can go ahead and merge the change into >> > > release/1.7.0 >> > > > > > > branch too when you merge the PR. Please do close the fixed >> > version >> > > > in >> > > > > > the >> > > > > > > JIRA as 1.7.0 >> > > > > > > >> > > > > > > Regards >> > > > > > > Nabarun Nag >> > > > > > > >> > > > > > > >> > > > > > > On Wed, Sep 12, 2018 at 2:50 PM Alexander Murmann < >> > > > amurm...@pivotal.io >> > > > > > >> > > > > > > wrote: >> > > > > > > >> > > > > > > > While there is a workaround this looks like a highly visible >> > bug >> > > > > with a >> > > > > > > > fairly safe fix. I am in favor of merging, since the branch >> is >> > > > still >> > > > > > > > distressed anyways. >> > > > > > > > >> > > > > > > > Other opinions? >> > > > > > > > >> > > > > > > > On Wed, Sep 12, 2018 at 2:29 PM, Jinmei Liao < >> > jil...@pivotal.io> >> > > > > > wrote: >> > > > > > > > >> > > > > > > > > Should we include the fix for GEODE-5727 in the 1.7 >> release >> > as >> > > > > well? >> > > > > > > > > >> > > > > > > > > Without the fix, the command "export cluster-config >> > > > > > > > --zip-file-name=x.zip" >> > > > > > > > > would fail with NPE, user has to use "export >> cluster-config >> > > > > > > > > --zip-file-name=./x.zip" in order for export to work. >> > > > > > > > > >> > > > > > > > > PR for this fix is ready and could be merged soon. >> > > > > > > > > >> > > > > > > > > Jinmei >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > On Wed, Sep 12, 2018 at 11:12 AM Patrick Rhomberg < >> > > > > > > prhomb...@apache.org> >> > > > > > > > > wrote: >> > > > > > > > > >> > > > > > > > > > I'm not sure PR 2457 will help with an ignored >> .buildinfo, >> > > but >> > > > > I'm >> > > > > > > not >> > > > > > > > > sure >> > > > > > > > > > as to why .buildinfo would be getting ignored by >> anything, >> > > > > either. >> > > > > > > > > > >> > > > > > > > > > PR 2457 deals with the still-needs-to-be-renamed >> > > > > > > > > GemFireVersion.properties >> > > > > > > > > > file and when it is generated. Previously, it was >> whenever >> > > the >> > > > > git >> > > > > > > > index >> > > > > > > > > > changed, which was too frequent. Not it is whenever the >> > > source >> > > > > > > > > parameters >> > > > > > > > > > are passed on the command-line with the build, which has >> > > > > presented >> > > > > > > > issues >> > > > > > > > > > outside the Concourse pipeline. PR 2457 splits the >> > > difference, >> > > > > > > > > > regenerating the file anytime the SHA changes. >> > > > > > > > > > >> > > > > > > > > > The only interaction with .buildinfo that I can see is >> that >> > > if >> > > > > the >> > > > > > > > build >> > > > > > > > > > was run on a machine that was missing git, it would >> attempt >> > > to >> > > > > read >> > > > > > > > > values >> > > > > > > > > > instead from .buildinfo when creating the >> > > > > GemFireVersion.properties >> > > > > > > > file. >> > > > > > > > > > >> > > > > > > > > > I guess I don't fully understand the problem Anthony has >> > > called >> > > > > > out. >> > > > > > > > > Where >> > > > > > > > > > is it exactly that information previously gathered from >> > > > > .buildinfo >> > > > > > is >> > > > > > > > now >> > > > > > > > > > missing? And are we certain that it was indeed pulling >> > from >> > > > > > > .buildinfo >> > > > > > > > > and >> > > > > > > > > > not the aforementioned GemFireVersion.properties? >> > > > > > > > > > >> > > > > > > > > > On Wed, Sep 12, 2018 at 10:11 AM, Alexander Murmann < >> > > > > > > > amurm...@pivotal.io >> > > > > > > > > > >> > > > > > > > > > wrote: >> > > > > > > > > > >> > > > > > > > > > > Hi everyone, >> > > > > > > > > > > >> > > > > > > > > > > It seems like that PR doesn't address the missing SHA >> > issue >> > > > > > either >> > > > > > > > and >> > > > > > > > > I >> > > > > > > > > > am >> > > > > > > > > > > not aware of any proposals to properly fix this. How >> > viable >> > > > is >> > > > > it >> > > > > > > to >> > > > > > > > > > revert >> > > > > > > > > > > the relevant Gradle build changes on support/1.7? >> > > > > > > > > > > We could continue make the new Gradle approach work >> with >> > > our >> > > > > > > release >> > > > > > > > > > > process on develop and hopefully release 1.8 with >> these >> > > > > changes. >> > > > > > > > > > > >> > > > > > > > > > > Are there any other proposals to unblock this? >> > > > > > > > > > > >> > > > > > > > > > > On Tue, Sep 11, 2018 at 5:41 PM, Anthony Baker < >> > > > > > aba...@pivotal.io> >> > > > > > > > > > wrote: >> > > > > > > > > > > >> > > > > > > > > > > > Slight clarification—the issue I mentioned is when a >> > user >> > > > > > builds >> > > > > > > > > Geode >> > > > > > > > > > > > from the source distribution. The source >> distribution >> > > that >> > > > > the >> > > > > > > > > release >> > > > > > > > > > > > manager creates has the correct .buildinfo file, >> it’s >> > > just >> > > > > > > ignored >> > > > > > > > by >> > > > > > > > > > the >> > > > > > > > > > > > build. In short, the release manager can’t work >> around >> > > the >> > > > > > > > problem. >> > > > > > > > > > > > >> > > > > > > > > > > > Does [1] help with this? >> > > > > > > > > > > > >> > > > > > > > > > > > Anthony >> > > > > > > > > > > > >> > > > > > > > > > > > [1] https://github.com/apache/geode/pull/2457 < >> > > > > > > > > > > https://github.com/apache/ >> > > > > > > > > > > > geode/pull/2457> >> > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > > > On Sep 11, 2018, at 3:16 PM, Alexander Murmann < >> > > > > > > > > amurm...@pivotal.io> >> > > > > > > > > > > > wrote: >> > > > > > > > > > > > > >> > > > > > > > > > > > > What's the consensus on the version info issue >> > Anthony >> > > is >> > > > > > > calling >> > > > > > > > > > out? >> > > > > > > > > > > > Does >> > > > > > > > > > > > > anyone have a proposal for fixing this for this >> > > release? >> > > > > > Should >> > > > > > > > > > Nabarun >> > > > > > > > > > > > as >> > > > > > > > > > > > > the release manager manually correct this for the >> > > release >> > > > > and >> > > > > > > we >> > > > > > > > > > find a >> > > > > > > > > > > > > permanent solution for 1.8? >> > > > > > > > > > > > > >> > > > > > > > > > > > > On Mon, Sep 10, 2018 at 12:33 PM, Anthony Baker < >> > > > > > > > aba...@pivotal.io >> > > > > > > > > > >> > > > > > > > > > > > wrote: >> > > > > > > > > > > > > >> > > > > > > > > > > > >> Unfortunately it would require a fix to the >> > build—it’s >> > > > not >> > > > > > > about >> > > > > > > > > > > > producing >> > > > > > > > > > > > >> the release candidate. It’s when a user builds >> from >> > > the >> > > > > > source >> > > > > > > > > > release >> > > > > > > > > > > > that >> > > > > > > > > > > > >> the version info is ignored. >> > > > > > > > > > > > >> >> > > > > > > > > > > > >> Anthony >> > > > > > > > > > > > >> >> > > > > > > > > > > > >>> On Sep 10, 2018, at 10:02 AM, Nabarun Nag < >> > > > > n...@apache.org >> > > > > > > >> > > > > > > > > wrote: >> > > > > > > > > > > > >>> >> > > > > > > > > > > > >>> Hello Anthony, >> > > > > > > > > > > > >>> >> > > > > > > > > > > > >>> I plan to do that while creating the release >> > > candidate. >> > > > > If >> > > > > > > > there >> > > > > > > > > > are >> > > > > > > > > > > no >> > > > > > > > > > > > >>> concerns raised on the release branch, I will >> start >> > > > with >> > > > > > the >> > > > > > > > > > process >> > > > > > > > > > > > >> soon. >> > > > > > > > > > > > >>> >> > > > > > > > > > > > >>> Regards >> > > > > > > > > > > > >>> Nabarun Nag >> > > > > > > > > > > > >>> >> > > > > > > > > > > > >>>> On Mon, Sep 10, 2018 at 8:51 AM Anthony Baker < >> > > > > > > > > aba...@pivotal.io> >> > > > > > > > > > > > >> wrote: >> > > > > > > > > > > > >>>> >> > > > > > > > > > > > >>>> Looks good Naba! Only thing I see right now is >> > that >> > > > > > > building >> > > > > > > > > from >> > > > > > > > > > > the >> > > > > > > > > > > > >>>> source distribution does not use the .buildinfo >> > > file, >> > > > > > > leaving >> > > > > > > > > the >> > > > > > > > > > > > >> version >> > > > > > > > > > > > >>>> string empty. >> > > > > > > > > > > > >>>> >> > > > > > > > > > > > >>>> Anthony >> > > > > > > > > > > > >>>> >> > > > > > > > > > > > >>>> >> > > > > > > > > > > > >>>>> On Sep 7, 2018, at 9:15 AM, Nabarun Nag < >> > > > > n...@apache.org >> > > > > > > >> > > > > > > > > wrote: >> > > > > > > > > > > > >>>>> >> > > > > > > > > > > > >>>>> CORRECTION: if '*no*' concerns are raised, we >> > will >> > > > > start >> > > > > > > with >> > > > > > > > > the >> > > > > > > > > > > > >> voting >> > > > > > > > > > > > >>>>> for the release candidate soon. >> > > > > > > > > > > > >>>>> >> > > > > > > > > > > > >>>>> Regrads >> > > > > > > > > > > > >>>>> Nabarun Nag >> > > > > > > > > > > > >>>>> >> > > > > > > > > > > > >>>>>> On Fri, Sep 7, 2018 at 9:08 AM Nabarun Nag < >> > > > > > > n...@pivotal.io >> > > > > > > > > >> > > > > > > > > > > wrote: >> > > > > > > > > > > > >>>>>> >> > > > > > > > > > > > >>>>>> Hello Geode Dev Community, >> > > > > > > > > > > > >>>>>> >> > > > > > > > > > > > >>>>>> We have created a new release branch for >> Apache >> > > > Geode >> > > > > > > 1.7.0 >> > > > > > > > - >> > > > > > > > > > > > >>>>>> "release/1.7.0" >> > > > > > > > > > > > >>>>>> >> > > > > > > > > > > > >>>>>> Previous branch was deleted and has been >> > replaced >> > > > > with a >> > > > > > > > fresh >> > > > > > > > > > > one. >> > > > > > > > > > > > >>>>>> >> > > > > > > > > > > > >>>>>> Please do review and raise any concern with >> the >> > > > > release >> > > > > > > > > branch. >> > > > > > > > > > > > >>>>>> >> > > > > > > > > > > > >>>>>> If concerns are raised, we will start with >> the >> > > > voting >> > > > > > for >> > > > > > > > the >> > > > > > > > > > > > release >> > > > > > > > > > > > >>>>>> candidate soon. >> > > > > > > > > > > > >>>>>> >> > > > > > > > > > > > >>>>>> Regards >> > > > > > > > > > > > >>>>>> Nabarun Nag >> > > > > > > > > > > > >>>>>> >> > > > > > > > > > > > >>>> >> > > > > > > > > > > > >>>> -- >> > > > > > > > > > > > >>> Regards >> > > > > > > > > > > > >>> Nabarun Nag >> > > > > > > > > > > > >> >> > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > -- >> > > > > > > > > Cheers >> > > > > > > > > >> > > > > > > > > Jinmei >> > > > > > > > > >> > > > > > > > >> > > > > > > -- >> > > > > > > Regards >> > > > > > > Nabarun Nag >> > > > > > > >> > > > > > >> > > > > -- >> > > > > Regards >> > > > > Nabarun Nag >> > > > > >> > > > >> > > -- >> > > Regards >> > > Nabarun Nag >> > > >> > >> >
