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.

Sign up for GitHub

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

Modify PrepFunctionalTests.sh to work out of the output directories #427

Merged
merged 2 commits into from
Oct 26, 2018
Merged

Modify PrepFunctionalTests.sh to work out of the output directories #427

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
90b6477
Modify PrepFunctionalTests.sh to work out of the publish directory
jrbriggs Oct 25, 2018
583a33b
Relocated Git installer to BuildOuptut
jrbriggs Oct 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Scripts/Mac/BuildGVFSForMac.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,13 @@ echo 'Copying native binaries to Publish directory...'
cp $VFS_OUTPUTDIR/GVFS.Native.Mac/Build/Products/$CONFIGURATION/GVFS.ReadObjectHook $VFS_PUBLISHDIR || exit 1
cp $VFS_OUTPUTDIR/GVFS.Native.Mac/Build/Products/$CONFIGURATION/GVFS.VirtualFileSystemHook $VFS_PUBLISHDIR || exit 1

echo 'Copying Git installer to Publish directory...'
GITPUBLISH=$VFS_PUBLISHDIR/Git
if [[ -d $GITPUBLISH ]] ; then
rm -rf $GITPUBLISH
fi
mkdir $GITPUBLISH
cp $GITPATH $GITPUBLISH
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it necessary to actually copy? Couldn't we just update the init script to set the GITPATH variable appropriately?

Two concerns with this:

  • build+publish+unit tests is already getting to be too long. Adding a copy of a large package is just going to add to that. Maybe do the copy only if it's not already there?
  • The VFS_PUBLISHDIR currently contains just the outputs of dotnet publish GVFS.sln. With this change, that publish folder will now contain different contents if I build+publish from VS or build+publish with this script. So if we do need to copy, I'd say copy the Git package to a parallel folder, not inside the same publish folder.

Thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't we just update the init script to set the GITPATH variable appropriately?

I'm not a fan of that, since it means we need to sweep up the packages (which I view as build input) and pass it between jobs. I think that if we need something in a package available to end users, the build should promote it to an output folder.

build+publish+unit tests is already getting to be too long. Adding a copy of a large package is just going to add to that. Maybe do the copy only if it's not already there?

Makes sense to me. I'll make that change.

So if we do need to copy, I'd say copy the Git package to a parallel folder, not inside the same publish folder.

Also makes sense. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To add one more element here - today I can do a clean build+publish from VS, then run the prep script, and then run the functional tests.

With this change, we would now require that the build happen via this build script, at least once. It would be nice to keep the ability to build from VS if possible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sanoursa and I agreed offline that, unfortunately, clean build+publish from VS already doesn't work, and the build script is already a prereq. I'm adding to the problem here. We'll move more of this into the build phase itself in subsequent refactorings, but it's deferred while we work on moving to yaml pipelines.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me. Note that I plan to spend some time next week digging into getting our build working well in VSMac (or at least understanding why things don't work well today).


echo 'Running VFS for Git unit tests...'
$VFS_PUBLISHDIR/GVFS.UnitTests || exit 1
9 changes: 4 additions & 5 deletions Scripts/Mac/PrepFunctionalTests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@
# Ensure the kext isn't loaded before installing Git
$VFS_SRCDIR/ProjFS.Mac/Scripts/UnloadPrjFSKext.sh

# Install GVFS-aware Git (that was downloaded by the build script)
GITVERSION="$($SCRIPTDIR/GetGitVersionNumber.sh)"
GITDIR=$VFS_PACKAGESDIR/gitformac.gvfs.installer/$GITVERSION/tools
if [[ ! -d $GITDIR ]]; then
# Install GVFS-aware Git (that was published by the build script)
GITPUBLISH=$VFS_PUBLISHDIR/Git
if [[ ! -d $GITPUBLISH ]]; then
echo "GVFS-aware Git package not found. Run BuildGVFSForMac.sh and try again"
exit 1
fi
hdiutil attach $GITDIR/*.dmg || exit 1
hdiutil attach $GITPUBLISH/*.dmg || exit 1
GITPKG="$(find /Volumes/Git* -type f -name *.pkg)" || exit 1
sudo installer -pkg "$GITPKG" -target / || exit 1
hdiutil detach /Volumes/Git*
Expand Down
6 changes: 0 additions & 6 deletions Scripts/Mac/RunFunctionalTests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,6 @@ if [ -z $CONFIGURATION ]; then
CONFIGURATION=Debug
fi

SCRIPTDIR=$(dirname ${BASH_SOURCE[0]})

SRCDIR=$SCRIPTDIR/../..
ROOTDIR=$SRCDIR/..
PUBLISHDIR=$ROOTDIR/Publish

sudo mkdir /GVFS.FT
sudo chown $USER /GVFS.FT

Expand Down