Skip to content

Conversation

@lassoan
Copy link
Contributor

@lassoan lassoan commented Mar 20, 2015

Through superbuild mechanism we've been automatically pulling the latest versions from the repository and it worked great for years. Now we don't use superbuild anymore but we would like to keep pulling the latest version from SlicerIGT master automatically, so changed revision identifier from a specific git hash to master.

Always use the latest version in master. Through superbuild mechanism we've been automatically pulling the latest versions from the repository and it worked great.
Now we don't use superbuild anymore to simplify multiple module updates (all modules are in the same repository) but we would like to keep using the latest version automatically
(without asking for a manual git hash update each time we change something).
@pieper
Copy link
Member

pieper commented Mar 20, 2015

Hi Andras -

It has been our policy not to support this. I know Jc felt it was important to have an actual hash and not a branch or tag and he was convincing in the discussion. One reason is to ensure that we don't have version skew across the different nightly builds. @jcfr would you like to comment?

-Steve

@lassoan
Copy link
Contributor Author

lassoan commented Mar 20, 2015

We've been doing this since 2012 Summer through superbuild mechanism. First we did this accidentally (the superbuild pulled the latest master versions of all external projects by default), but then it worked really well, so we kept using this. It makes a huge difference that any change that we commit gets into the next nightly build automatically.
We merged all the individual modules into one repository, so we don't need superbuild anymore, but we would like to keep this efficient workflow.

@pieper
Copy link
Member

pieper commented Mar 20, 2015

Yes, that's the counter argument ; )

We used to allow this in slicer3 as well. Let's see what Jc thinks now
that he's been manually approving extension pull requests for a while.

Obviously there's nothing that prevents circumventing the policy through a
superbuild or something similar.

On Fri, Mar 20, 2015 at 1:46 PM, Andras Lasso notifications@github.com
wrote:

We've been doing this since 2012 Summer through superbuild mechanism.
First we did this accidentally (the superbuild pulled the latest master
versions of all external projects by default), but then it worked really
well, so we kept using this. It makes a huge difference that any change
that we commit gets into the next nightly build automatically.
We merged all the individual modules into one repository, so we don't need
superbuild anymore, but we would like to keep this efficient workflow.


Reply to this email directly or view it on GitHub
#924 (comment)
.

The information in this e-mail is intended only for the person to whom it
is
addressed. If you believe this e-mail was sent to you in error and the
e-mail
contains patient information, please contact the Partners Compliance
HelpLine at
http://www.partners.org/complianceline . If the e-mail was sent to you in
error
but does not contain patient information, please contact the sender and
properly
dispose of the e-mail.

@lassoan
Copy link
Contributor Author

lassoan commented Mar 23, 2015

@jcfr Please comment/approve. We would need this SlicerIGT extension update.

@jcfr
Copy link
Member

jcfr commented Mar 23, 2015

Hi Andras,

(( Just came back from travelling. ))

As mentioned by Steve, the idea behind explicitly specifying version number
is to be able to clearly identify which version of the code base lead to
which set of binaries and results. For sake of transparency and
reproducibility, I think this is important. More details are highlighted in
[1]

Generalizing the approach that has been used in SlicerIGT means that the
idea described above does not apply anymore. To ensure tracking of the
source revision, I am thinking about the following solutions:

(a) Trade-off approach: expect an exact revision only for release.
Development version (aka Nightly) would be based on "origin/master" for
every extension. Stable version would always specify the revision.

(b) Streamlined approach: after ensuring confirming an extension complied
with our license requirements, pull request would be automatically
integrated to the index by the "slicerbot" github user [2]

(c) "Extension/module with Version" approach: we update the requirement by
allowing the use of "origin/master" only for extension maintaining a
version scheme. We also extend the Slicer module interface to have a
"version" property.

[1]
https://github.com/QIICR/ProjectIssuesAndWiki/wiki/Software-provenance-for-SR#for-3d-slicer-based-data-provenance-we-should-have

[2]
http://wiki.slicer.org/slicerWiki/index.php/Documentation/Nightly/Developers/Build_system/SlicerBot

On Mon, Mar 23, 2015 at 3:25 PM, Andras Lasso notifications@github.com
wrote:

@jcfr https://github.com/jcfr Please comment/approve. We would need
this SlicerIGT extension update.


Reply to this email directly or view it on GitHub
#924 (comment)
.

+1 919 869 8849

@lassoan
Copy link
Contributor Author

lassoan commented Mar 24, 2015

Extensions (with possible updates) are re-built for the stable Slicer release every night, so Slicer version number or git hash never specifies the extensions' revisions. So, to reproduce a software configuration we already need the revision of Slicer, the list of all installed extensions, and the revision of each extension (and probably also operating system, bitness, ini file options). Therefore, picking up the latest revision for an extension each night does not affect software provenance.

I would accept any of the a-b-c options.
For option c) it's not clear what would be the version scheme. If it's the SVN revision or git hash then I think we have this already; if it is something that is generated manually then there is a chance that errors are made (e.g., changes are made but the version is not updated).

@jcfr
Copy link
Member

jcfr commented Mar 24, 2015

Following today's hangout, we reach the conclusion that specifying origin/master or origin/4.4 is a valid solution to ensure automatic update the extension.

Action items:

  • update wiki documentation specifying that this should be done for automatic update. The "older" appraoch is style valid.

@pieper
Copy link
Member

pieper commented Mar 25, 2015

Makes good sense - thanks for hashing through the details!

On Tue, Mar 24, 2015 at 2:21 PM, Jean-Christophe Fillion-Robin <
notifications@github.com> wrote:

Following today's hangout, we reach the conclusion that specifying
origin/master or origin/4.4 is a valid solution to ensure automatic
update the extension.

Action items:

  • update wiki documentation specifying that this should be done for
    automatic update. The "older" appraoch is style valid.


Reply to this email directly or view it on GitHub
#924 (comment)
.

The information in this e-mail is intended only for the person to whom it
is
addressed. If you believe this e-mail was sent to you in error and the
e-mail
contains patient information, please contact the Partners Compliance
HelpLine at
http://www.partners.org/complianceline . If the e-mail was sent to you in
error
but does not contain patient information, please contact the sender and
properly
dispose of the e-mail.

@lassoan
Copy link
Contributor Author

lassoan commented Mar 28, 2015

Guys, we all agreed but finally nobody merged the pull request. Please someone merge it, as the current version will have build errors due to Slicer core changes. Thanks!

jcfr added a commit that referenced this pull request Mar 28, 2015
Updated SlicerIGT extension version
@jcfr jcfr merged commit be77787 into Slicer:master Mar 28, 2015
@jcfr
Copy link
Member

jcfr commented Mar 28, 2015

All set.

I also just gave you and Csaba push access.

Have a good weekend,
Jc

On Sat, Mar 28, 2015 at 12:30 AM, Andras Lasso notifications@github.com
wrote:

Guys, we all agreed but finally nobody merged the pull request. Please
someone merge it, as the current version will have build errors due to
Slicer core changes. Thanks!


Reply to this email directly or view it on GitHub
#924 (comment)
.

+1 919 869 8849

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants