Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create release source artifact containing all submodules #1712

Merged
merged 15 commits into from Feb 5, 2014
Merged

Create release source artifact containing all submodules #1712

merged 15 commits into from Feb 5, 2014

Conversation

ghost
Copy link

@ghost ghost commented Nov 5, 2013

Testing:

./build.py release-src

This will create a source release. If the repo is tagged, you should get the correct release version here.

The resulting zip file should contain

  • all submodules
  • correct executable flags on all files executable in the git tree
  • components/antlib/resources/gitversion.xml containing the release version numbers

The unpacked zip file should build all the same targets as for the git repo, the only exception being that the release-src target will not work, so ./build.py release-all will exclude the source zip when run outside git. We could remove this from release-all so that it's only for binary artifacts.

@manics
Copy link
Member

manics commented Nov 6, 2013

Generally looks good, source is now in a directory instead at the top level.

Why not replace OMERO.source-5.0.0-beta1-1304-1e8b196-b478.zip completely instead of a separate openmicroscopy-5.0.0-beta1-1304-1e8b196-b478.zip?

build.py has lost its execute permissions

Comparing s local checkout of scc/merge/develop/latest with http://hudson.openmicroscopy.org.uk/job/OMERO-merge-develop/lastSuccessfulBuild/artifact/src/target/openmicroscopy-5.0.0-beta1-1304-1e8b196-b478.zip (using diff -b because my mac has weird issue with line endings):

$ diff -rqb ~/work/openmicroscopy.dev/ openmicroscopy-5.0.0-beta1-1304-1e8b196-b478
Only in /Users/simon/work/openmicroscopy.dev/: .git
Only in /Users/simon/work/openmicroscopy.dev/components/bioformats: .git
Files /Users/simon/work/openmicroscopy.dev/components/bioformats/lib/libJWlz.so and openmicroscopy-5.0.0-beta1-1304-1e8b196-b478/components/bioformats/lib/libJWlz.so differ
Only in /Users/simon/work/openmicroscopy.dev/components/rendering/src/omeis: CVS
Only in /Users/simon/work/openmicroscopy.dev/components/tools/OmeroPy/scripts: .git
Only in /Users/simon/work/openmicroscopy.dev/docs/sphinx: .git

components/bioformats/lib/libJWlz.so is a symlink to libJWlz-1.4.0.so, in the zip it becomes a text file:

cat openmicroscopy-5.0.0-beta1-1304-1e8b196-b478/components/bioformats/lib/libJWlz.so
libJWlz-1.4.0.so

but this probably shouldn't be in the repo in the first place and it seems to build fine anyway. Other differences can be ignored.

@joshmoore
Copy link
Member

Keeping the OMERO.source was my not knowing what is currently using it. I'm also not sure of the name openmicroscopy-.... If we ever do rename this repo, that becomes a bit too all-encompassing.

@manics
Copy link
Member

manics commented Nov 6, 2013

Doesn't the lack of submodules prevent any functional use of OMERO.source?

@ghost
Copy link
Author

ghost commented Nov 6, 2013

If we rename the repo, then we can simply rename the archive at the same time.

I'll double-check why we're losing the executable bit. Looks like the ant zip task doesn't handle this.

WRT the missing bits, the .git directories are intentional; CVS is unintentional but OK I imagine--legacy from CVS days?

For JWlz.so, I opened #11655.

@ghost
Copy link
Author

ghost commented Nov 6, 2013

So looking at the ant tar/zip/untar/unzip tasks, it appears that they are completely incapable of handling posix permissions until they are updated to use the Java7 File API which introduces support for them. So if we want to run an external shell script to do the source generation, that's the best way to do it, but at the expense of portability. Mind you, we can't create an archive with correct permissions on Windows anyway, so maybe this isn't such a bad limitation. Thoughts?

@joshmoore
Copy link
Member

  • I'm not sure what CVS is about. Worth getting rid of?
  • re: naming -- looking at this more, I agree. Can we do s/openmicroscopy/OMERO.source/ for the archive names and be done with it? (This is different from the cpp.src file which was intended to provide the generated source, which this archive isn't doing)

@joshmoore
Copy link
Member

@rleigh-dundee : are you ok to make those changes?

@ghost
Copy link
Author

ghost commented Nov 14, 2013

The main outstanding blocker is how to keep the executable files executable in the generated zips. We could change from using ant to a shell or python script to run git and zip by hand (which will not work on Windows), but would correctly preserve the execute permissions [packing on Windows would destroy the permissions anyway, so it's not workable there anyway I think].

Insight uses the <zipfileset> mode attribute, but this only works if you know in advance which files have which permissions. We don't know, and we can't preserve them on unpack even if we could make use of this on repack as far as I can tell. We could potentially scan the git tree objects to find the executable blobs, but doing this in ant is likely a tall order.

CVS is an obsolete relic (had empty CVS/Entries); so can definitely be removed.

For the naming, I'm not sure what you mean here; I was suggesting to do the opposite and drop the .source entirely. Naming the source release of openmicroscopy.git "OMERO.source" is counter to what all but a few projects do for their source releases, and is counter to the expectations of people who would want to download and build this source (i.e. it's highly unconventional, and even if it only contained OMERO alone it would still be problematic). As a concrete example, it doesn't meet the package naming conventions used by Debian or RedHat/Fedora (and others), so all these potential downstream consumers would need to unnecessarily repack the archive to correct the name before they could use it.

Fedora does not permit dots as separators in a package name; Debian does not permit uppercase package names, so our being gratuitously different from the rest of the world here does make us incompatible with these two projects (and doubtless many others), so this does have a cost.

@joshmoore
Copy link
Member

re: permissions. There are far more than I would have expected:

$ for x in `git ls-files`; do test -x $x && echo ${x##*.}; done | sort | uniq -c
      4 bat
      1 bmp
      1 components/tools/OmeroPy/bin/omero
      1 components/tools/OmeroPy/scripts
     46 css
      2 cur
      1 docs/sphinx
     22 gif
      8 html
      1 ico
      4 jar
     29 java
      1 jpg
    153 js
      1 jsb2
      7 m
      3 md
      8 nsh
      1 nsi
      3 omero
     80 png
     79 py
      9 sh
      3 txt

Most likey the goal of this PR shouldn't be to make the new source zip completely build-able, since that's too much work. It didn't build previously, so adding the submodules will already be an improvement. I filed handling the exec bit as: https://trac.openmicroscopy.org.uk/ome/ticket/11593

@joshmoore
Copy link
Member

Re: zip naming -- I filed https://trac.openmicroscopy.org.uk/ome/ticket/11701

Let's not try to make such decisions here but get several people together to chat. For the moment, please make use of OMERO.source which will also keep jobs green.

@manics
Copy link
Member

manics commented Nov 18, 2013

re: permissions. There are far more than I would have expected:

How many of those are required, as opposed to permissions which were inadvertently committed?

The default build target actually works fine after making ./builld.py executable, server runs, however OMERO.web doesn't work due to some version number problems:
Error: Client version does not match server, please contact administrator.
So if anything I think this would be the next priority.

@joshmoore
Copy link
Member

@manics : not many are needed, I don't think.

@joshmoore
Copy link
Member

Ping.

@ghost
Copy link
Author

ghost commented Nov 25, 2013

@joshmoore My question about the execute perms which I've asked twice remains unanswered. We need the permissions preserving, and this isn't possible with ant. We could use either a simple shell script or a python script, for example, run git archive/unzip/zip to do this, but whichever we choose it's not going to be possible for this to (ever) work well on windows as is.

One possibility is that

git ls-tree -r HEAD | grep "^...755" | cut -f2
build.py
components/antlib/nsis/check_ice.nsh
components/antlib/nsis/check_java.nsh
components/antlib/nsis/check_nginx.nsh
components/antlib/nsis/check_pil.nsh
...

gives us a list of files which are executable in git. This would be a means of setting explicit permissions, which would be worth looking at. However, we would need to find a way to exclude these entries from the other lists. Doing this in ant will not be pleasant. It wouldn't be hard with shell or python. This might even work for windows--we can add the execute property with the python "zipfile" module, for example.

If it would be OK to move the zipfile creation out from ant to an external script, I'll be happy to do the above. Where would be the best place to put such a script?

@joshmoore
Copy link
Member

I have 3 different reactions (at least):

  1. It didn't build before and therefore perhaps we should be happy enough with where we are for the moment
  2. Perhaps we should generate a script (or patch build.py or similar) to fix this after the fact
  3. As @manics pointed out, we don't need many of the permissions and therefore an intermediate fix is likely possible.

Happy to discuss any of them, but since the rest of the build fixes have been ticketed and moved to a later milestone, I would tend to do 1 unless someone has tried 3 and knows it to be easy and feasible.

@joshmoore
Copy link
Member

@rleigh-dundee : is this something you want to come back to now?

@ghost
Copy link
Author

ghost commented Jan 15, 2014

Yes. I'll redo this as a python script. shoehorning this into ant works up to a point, but really just isn't good enough.

@ghost
Copy link
Author

ghost commented Jan 15, 2014

Given the age of the branch, I've rebased it onto current develop. I've added a separate python script to do the archive creation which doesn't have the limitations of the ant-based approach. This now creates a source archive containing all submodules and with correct permissions across the board.

@joshmoore
Copy link
Member

exclude removed; also tested locally and all builds fine. Only (known) downside is:

jamoore@blue:/tmp/FOO/openmicroscopy-5.0.0-rc1-DEV$ ./build.py version
Buildfile: /tmp/FOO/openmicroscopy-5.0.0-rc1-DEV/build.xml

version:
UNKNOWN
${version.describe}-ice35

@joshmoore
Copy link
Member

And trying to release-src from a source release also doesn't work:

(ome1)jamoore@blue:/tmp/FOO/openmicroscopy-5.0.0-rc1-DEV$ OMERO_BRANCH=test bash docs/hudson/OMERO.sh
...

release-src:
fatal: Not a git repository (or any of the parent directories): .git
Releasing openmicroscopy-UNKNOWN
Traceback (most recent call last):
  File "/tmp/FOO/openmicroscopy-5.0.0-rc1-DEV/components/antlib/scripts/source-archive", line 41, in <module>
    raise Exception('Failed to create git base archive')
Exception: Failed to create git base archive

BUILD FAILED
/tmp/FOO/openmicroscopy-5.0.0-rc1-DEV/build.xml:443: exec returned: 1

Total time: 3 minutes 12 seconds
(ome1)jamoore@blue:/tmp/FOO/openmicroscopy-5.0.0-rc1-DEV$ 

Which also seems acceptable. Assuming there are no problems with the builds tomorrow, I'd say let's merge this and make other minor modifications as need be.

@ghost
Copy link
Author

ghost commented Feb 3, 2014

Version number is now preserved by release-src.

@ghost
Copy link
Author

ghost commented Feb 3, 2014

Having it in 5.0 would be ideal. I'll be happy to backport as soon as we know all the targets work correctly. Further down the line, many of the jobs which use git clones could be converted to download the source zip from an upstream job, which reduce the diskspace and download times significantly, as well as ensure that the release source is always buildable.

@sbesson
Copy link
Member

sbesson commented Feb 3, 2014

👍 for backporting to 5.0. Happy to test this tomorrow.
A couple of immediate thoughts:

  • <target name="version" depends="version-embed,version-git"/> in the case of a Git repo, does that involve that if gitversion.xml exists, it will take priority over the git describe output?
  • related to the previous question, thoughts about moving gitversion.xml to e.g. target so that ./build.py clean can wipe it out (thinking of our daily job specifically)
  • is is timely to ask @gusferguson if he can generate a shiny release source code icon to expose this new artifact under http://downloads.openmicroscopy.org/omero/5.0.0-rc1/#code ?

@ghost
Copy link
Author

ghost commented Feb 3, 2014

@sbesson to answer your questions:

  • if gitversion.xml exists, it will take priority over the git output. However, note that it will never exist in a git tree--it's only created inside the release zip file. If one is ever created by hand, it will be cleanable by git clean.
  • gitversion.xml should never be cleaned--it's a release file, and there's no way to recreate it, so it would break any future build.py use. From the POV of jobs building from git, this won't be a problem since they will never encounter this file.
  • An icon for source downloads would be great; this should soon be ready to be used.

@joshmoore
Copy link
Member

@rleigh-dundee : would you mind enabling release-src either as a part of release-all in build.xml or in docs/hudson/OMERO.sh? This should get it into every daily build. Thanks.

@joshmoore
Copy link
Member

NB: ./build.py release-src && cd target && unzip target/openmicroscopy*zip && cd openmicroscopy* && ./build.py release-src clearly fails with:

BUILD FAILED
/opt/hudson/workspace/OMERO-5.1-merge-build/src/target/openmicroscopy-5.1.0-m0-DEV/build.xml:448: Releasing is only possible from a git repository

@ghost
Copy link
Author

ghost commented Feb 4, 2014

@joshmoore Does this work if you unpack outside the git repo?

@joshmoore
Copy link
Member

Nope:

[hudson@ome-c6100-3 openmicroscopy-5.1.0-m0-DEV]$ pwd
/tmp/openmicroscopy-5.1.0-m0-DEV
[hudson@ome-c6100-3 openmicroscopy-5.1.0-m0-DEV]$ ./build.py release-src
Buildfile: /tmp/openmicroscopy-5.1.0-m0-DEV/build.xml

BUILD FAILED
/tmp/openmicroscopy-5.1.0-m0-DEV/build.xml:448: Releasing is only possible from a git repository

Total time: 0 seconds

but just to be clear: this is my expected, preferred behavior.

@ghost
Copy link
Author

ghost commented Feb 4, 2014

Ah, OK. I've added release-src to OMERO.sh now.

@joshmoore
Copy link
Member

Nice. We can look at the artifacts of the next build and then merge. (In the future we'll probably need this in OMERO.bat as well)

@ghost
Copy link
Author

ghost commented Feb 4, 2014

OK. Note that the source-archive script was also copied into the bioformats PR. Once that's merged, we can move over to using that in openmicroscopy as well.

@joshmoore
Copy link
Member

joshmoore added a commit that referenced this pull request Feb 5, 2014
Create release source artifact containing all submodules
@joshmoore joshmoore merged commit 119dcfb into ome:develop Feb 5, 2014
@sbesson
Copy link
Member

sbesson commented Feb 24, 2014

--rebased-to #2064

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants