Add some shell scripts to help set up developer environment #16

adammoody · 2017-12-09T01:39:55Z

This adds a few shell scripts one can execute to prepare the environment for developers. I always forget the commands, so it helps to encode them in a file. This should help ensure we're working from a similar starting point.

Description

Adds a few scripts and a README in the setup/ directory. One script fetches and builds autotools, so we're working from the same set. Another downloads and builds leveldb. The third builds unifycr.

Motivation and Context

How Has This Been Tested?

Types of changes

Bug fix (non-breaking change which fixes an issue)
[x ] New feature (non-breaking change which adds functionality)
Performance enhancement (non-breaking change which improves efficiency)
Code cleanup (non-breaking change which makes code smaller or more readable)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation (a change to man pages or other documentation)

Checklist:

My code follows the UnifyCR code style requirements.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.
All commit messages are properly formatted.

adammoody · 2017-12-09T01:40:23Z

This is not ready yet, just posting it here so others can see where I'm going with it

nedbass · 2017-12-11T19:38:48Z

Thanks for posting this @adammoody!

My feeling is that these scripts are too specific to your workflow to serve as "official" developer documentation. If the general directions for developers isn't as simple as

... install dependencies from packages
./autogen.sh
./configure
make

then we're doing something wrong. If you want to have them in-tree for convenience, my suggestion is to put them under a top-level "contrib" directory. This sends a signal that they're not an officially maintained part of the tree.

adammoody · 2017-12-11T23:37:02Z

Moved these to scripts directory for now, instead of setup. The contrib directory as recommended by @nedbass is being used by our configure, so I didn't want to step on anything there.

nedbass · 2017-12-12T00:04:57Z

The contrib directory as recommended by @nedbass is being used by our configure

That seems like a build system bug. Can you share some details about what you ran into? There's no existing contrib directory in the tree. Are you saying that one appeared after you run configure?

adammoody · 2017-12-12T06:21:25Z

Ready for review... the autogen.sh was specifiying "aclocal -I contrib/aclocal" and throwing an error about not finding the directory. I changed that to "m4" instead on the build PR, which fixes the warning. So it doesn't need contrib. I've left the setup scripts in the scripts subdirectory.

nedbass · 2017-12-12T18:11:27Z

I'd still much rather see this go in a contrib/setup directory if we're going to merge it. This documents a very non-standard approach to setting up an open source development environment, and I wouldn't want people considering getting involved with the project to think this is the recommended procedure. I think we'd be much better served by creating a proper build and packaging system for UnifyCR.

adammoody · 2017-12-12T19:59:52Z

@nedbass, I moved the setup scripts back to contrib. I think something like spack will help with most of this, but this is just meant as a temporary solution so we can all build while waiting for a better method.

nedbass · 2017-12-12T20:26:35Z

Thanks for making that change @adammoody. To address the checkpatch warnings about file permissions, I'll submit a PR to add an exemption for the contrib directory in checkpatch.pl.

nedbass · 2017-12-12T20:36:05Z

contrib/README.md

+# Prepare developer environment
+
+Commands to set up a developer environment for UnifyCR.
+Requires that gcc v4.9.3+ is in your path:


It would be good to add some explanation about why someone might use these scripts, e.g.

This procedure is provided for users working on systems where the required tools and dependencies are not available in packaged form. Most users should use the standard build process documented in the UnifyCR README.

This README should probably be renamed to something like README.buildme.md since other unrelated files may eventually live in contrib.

nedbass · 2017-12-13T00:14:11Z

contrib/unifycr.astyle

+--break-one-line-headers
+
+# always use braces
+--add-braces


This will trigger complaints from checkpatch.pl about unnecessary braces for one-line conditional statements.

nedbass · 2017-12-13T00:18:07Z

contrib/buildme_dependencies

+pushd deps
+  installdir=`pwd`/install
+  rm -rf $installdir
+


Trailing whitespace in this file:

ERROR: trailing whitespace #342: FILE: contrib/buildme_dependencies:11: + $ ERROR: trailing whitespace #348: FILE: contrib/buildme_dependencies:17: + $ ERROR: trailing whitespace #351: FILE: contrib/buildme_dependencies:20: + $ ERROR: trailing whitespace #355: FILE: contrib/buildme_dependencies:24: + $ ERROR: trailing whitespace #359: FILE: contrib/buildme_dependencies:28: + $ ERROR: trailing whitespace #362: FILE: contrib/buildme_dependencies:31: + $ ERROR: trailing whitespace #373: FILE: contrib/buildme_dependencies:42: + $

nedbass · 2017-12-13T00:18:49Z

contrib/leveldb.pc

@@ -0,0 +1,10 @@
+#leveldb.pc
+prefix=__INSTALLDIR__
+exec_prefix=__INSTALLDIR__ 


trailing whitespace

nedbass · 2017-12-13T00:19:59Z

.gitignore

+autotools
+deps
+install
+setup/astyle*


should this be contrib/astyle?

nedbass · 2017-12-13T00:28:31Z

@adammoody would you be opposed to adding .sh extensions to the scripts to make the style checker happy? I'd prefer to avoid changing the stock kernel checkpatch.pl to add an exemption for contrib. We can always ignore those style errors if you do object to the extensions, but it may be a good idea to establish that naming convention.

edit setup README fix markdown in setup README rename setup to contrib add entries to gitignore for building from setup scripts autotools is using contrib, so go back to setup for now add astyle scripts style: force braces move setup files to scripts instead move setup files to scripts instead add instructions on how to run checkpatch script astyle options closer to checkpatch specify that newer gcc is needed for astyle avoid using custom autotools for now

adammoody · 2017-12-13T00:57:31Z

added .sh to the buildme scripts

nedbass · 2017-12-13T01:09:50Z

I noticed there are spack packages for astyle, gotcha, leveldb, autoconf, automake, and libtool. Why not use those?

adammoody · 2017-12-13T01:27:31Z

I've got a spack package started, and it does encode those dependencies. End users can use spack, and they won't need the buildme scripts at all. They are not for end users.

As developers, we might also be able to do that, in which case we can drop the buildme files altogether. They're just a hacky work around for now until we have something better.

nedbass · 2017-12-13T01:45:02Z

@adammoody sorry to be a nag about all of this. It's just that in my experience hacky workarounds tend to stick around longer than we plan. And it seems like spack already does what your buildme scripts do in a more clean and simple way. So why not skip the temporary hack and go straight to something we can live with for a while?

# This is the 1st commit message: # This is a combination of 14 commits. # This is the 1st commit message: initial behavior debug # This is the commit message #2: fix int/long/size_t/off_t confusion # This is the commit message LLNL#3: more size_t fixes # This is the commit message LLNL#4: code rationalization # This is the commit message LLNL#5: debug logging # This is the commit message LLNL#6: more debugging # This is the commit message LLNL#7: more understanding # This is the commit message LLNL#8: more reasonable constant values # This is the commit message LLNL#9: zero buffers before use # This is the commit message LLNL#10: more cleanup and callocs # This is the commit message LLNL#11: append uid to runstate file # This is the commit message LLNL#12: create runstate file after MPI_Init # This is the commit message LLNL#13: simplify client context copy # This is the commit message LLNL#14: debug meta_batch_get # This is the commit message #2: sysio-writeread example improvements # This is the commit message LLNL#3: tons more understanding and size_t cleanup # This is the commit message LLNL#4: build fixes # This is the commit message LLNL#5: build fixes # This is the commit message LLNL#6: build fixes # This is the commit message LLNL#7: build fixes # This is the commit message LLNL#8: build fixes # This is the commit message LLNL#9: build fixes # This is the commit message LLNL#10: simplify lio_listio aiocb_list setup # This is the commit message LLNL#11: clean up mdhim range_server_bget leveldb code # This is the commit message LLNL#12: add server include path # This is the commit message LLNL#13: add common include path # This is the commit message LLNL#14: remove old macros # This is the commit message LLNL#15: build fixes # This is the commit message LLNL#16: fix value computed not used warning # This is the commit message LLNL#17: fix printf formats # This is the commit message LLNL#18: build fixes # This is the commit message LLNL#19: fix UNIFYCR_VAL_SZ definition # This is the commit message LLNL#20: fix mismatch between client and server read request structs

kathrynmohror added the WIP label Dec 11, 2017

adammoody removed the WIP label Dec 12, 2017

nedbass suggested changes Dec 12, 2017

View reviewed changes

nedbass suggested changes Dec 13, 2017

View reviewed changes

adammoody closed this Dec 13, 2017

adammoody deleted the setup branch December 15, 2017 21:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add some shell scripts to help set up developer environment #16

Add some shell scripts to help set up developer environment #16

adammoody commented Dec 9, 2017

adammoody commented Dec 9, 2017

nedbass commented Dec 11, 2017

adammoody commented Dec 11, 2017

nedbass commented Dec 12, 2017

adammoody commented Dec 12, 2017

nedbass commented Dec 12, 2017

adammoody commented Dec 12, 2017

nedbass commented Dec 12, 2017

nedbass Dec 12, 2017 •

edited

nedbass Dec 13, 2017

nedbass Dec 13, 2017

nedbass Dec 13, 2017

nedbass Dec 13, 2017

nedbass commented Dec 13, 2017

adammoody commented Dec 13, 2017

nedbass commented Dec 13, 2017

adammoody commented Dec 13, 2017

nedbass commented Dec 13, 2017

Add some shell scripts to help set up developer environment #16

Add some shell scripts to help set up developer environment #16

Conversation

adammoody commented Dec 9, 2017

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

adammoody commented Dec 9, 2017

nedbass commented Dec 11, 2017

adammoody commented Dec 11, 2017

nedbass commented Dec 12, 2017

adammoody commented Dec 12, 2017

nedbass commented Dec 12, 2017

adammoody commented Dec 12, 2017

nedbass commented Dec 12, 2017

nedbass Dec 12, 2017 • edited

Choose a reason for hiding this comment

nedbass Dec 13, 2017

Choose a reason for hiding this comment

nedbass Dec 13, 2017

Choose a reason for hiding this comment

nedbass Dec 13, 2017

Choose a reason for hiding this comment

nedbass Dec 13, 2017

Choose a reason for hiding this comment

nedbass commented Dec 13, 2017

adammoody commented Dec 13, 2017

nedbass commented Dec 13, 2017

adammoody commented Dec 13, 2017

nedbass commented Dec 13, 2017

nedbass Dec 12, 2017 •

edited