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

Only symlink files that are tracked by git #95

Closed
andsens opened this issue Apr 22, 2014 · 22 comments
Closed

Only symlink files that are tracked by git #95

andsens opened this issue Apr 22, 2014 · 22 comments

Comments

@andsens
Copy link
Owner

andsens commented Apr 22, 2014

When a file is ignored by git in a castle, it should not be symlinked into $HOME.
This would also allow the removal of the .git folders and files pruning when listing the castle contents, since those files are ignored by git per default.

However: git check-ignore was only added in git 1.8.2, the change should only affect that version and upwards.

This would also fix #94.

@squaresurf
Copy link
Contributor

git check-ignore is a nice command, but could we use git ls-files instead? I'm not sure when ls-files was added, but I have a feeling its been around for a while.

@andsens
Copy link
Owner Author

andsens commented Apr 22, 2014

Ostensibly, yes. But you'd have to rewrite large portions of the loop since ls-files does not output directories. I am not entirely sure which one would be the cleaner and better approach.

The advantage of ls-files is that we don't have to check whether a file is ignored, saving us a call per iteration (which could make quite a difference depending on the repo).

@squaresurf
Copy link
Contributor

That is unfortunate that the loop would have to be rewritten. Luckily we do have unit tests that should help with that task. Is there any reason why we have to pay attention to directories? Could we take a git approach, where we create parent directories for files when necessary, but ignore directories otherwise? It seems to me that ls-files would simplify the logic quite a bit since you can just get the files that need to be symlinked, then symlink them. (I haven't dug into the code yet, so I may be over simplifying something there)

I'm going to look into when ls-files was added to git and I'll post the version here.

@squaresurf
Copy link
Contributor

I'm definitely over simplifying. I just remembered that there has to be a check if the file already exists. There are probably other things I'm over looking as well since I should just go and read the code.

@andsens
Copy link
Owner Author

andsens commented Apr 22, 2014

Luckily we do have unit tests that should help with that task.

Yes. This is where stuff like that really pays off :-)

Could we take a git approach, where we create parent directories for files when necessary, but ignore directories otherwise?

This might actually work! I was thinking about something along the same lines. It's all about re-implementing the linking table. The directories in the castle are represented through the left column. mkdir -p works like a charm even when a part of the path is a symlinked directory. The only challenge lies in recognizing legacy directory symlinks to the castle and file symlinks that have becom directory symlinks.

There are probably other things I'm over looking as well since I should just go and read the code.

Probably, the code is somewhat well-documented. If it's insufficient, please ask, maybe the comments only make sense to me.

@andsens andsens changed the title Don't symlink gitignored files in castles Only symlink files that are tracked by git Apr 22, 2014
@andsens
Copy link
Owner Author

andsens commented Apr 22, 2014

Updated the issue title from "Don't symlink gitignored files in castles" to "Only symlink files that are tracked by git", which makes more sense seeing how our approach has changed.

@squaresurf
Copy link
Contributor

It appears that ls-files has been around for a long time: git/git@9b23b4b

As you can see show-files is renamed to ls-files. That commit is contained in tags as old as v0.99.

What is the oldest version of git that homeshick supports? Do I need to do any version checking for this?

@andsens
Copy link
Owner Author

andsens commented Apr 23, 2014

Heh, that is... oold.
The reason why I added git backwards compatibility (i.e. < 1.6.5) was because of debian lenny, which was still running on some servers I used at that time. I don't think we need to go any lower than that.

p.s.: The testing branch I was talking about in #88 is now up and running. I also added some contribution guidelines, but I think you are pretty much following them all anyways :-)
The documentation about testing is now updated for bats as well.

@squaresurf
Copy link
Contributor

I figured that it has been around long enough to not worry about backwards compatibility, but just wanted to check before fixing this issue.

Your testing docs and CONTRIBUTING.md Looks great. Its nice when a project has clear guidelines like that. Thank you for starting the testing branch. I'll probably point to it when I'm not working on development.

I'm hoping to get to work on fixing this issue today :)

@squaresurf
Copy link
Contributor

Do you think that .gitignore files should be symlinked to the home dir? I think it should probably be allowed since its simpler and some people may be using that functionality to manage a global gitignore.

@andsens
Copy link
Owner Author

andsens commented Apr 23, 2014

I do. You can always avoid the symlink by putting it into the root of the castle, outside home/.

@squaresurf
Copy link
Contributor

Great point.

@squaresurf
Copy link
Contributor

I've run into a big issue when writing a test to allow symlinking items from within a .git folder. It appears that git doesn't let you add a folder named .git to the repo. This makes it so that our solution for managing git hooks won't work.

[~] $ git --version
git version 1.8.5.2 (Apple Git-48)
[~] $ git init gittest                                                                                                         
Initialized empty Git repository in ~/gittest/.git/                                                                
[~] $ cd gittest/                                                                                                              
[gittest] $ touch a                                                                                                            
[gittest] $ mkdir git                                                                                                          
[gittest] $ touch git/a                                                                                                        
[gittest] $ mkdir git/.git                                                                                                     
[gittest] $ touch git/.git/a                                                                                                   
[gittest] $ git add .                                                                                                          
[gittest] $ git status                                                                                                         
On branch master                                                                                                               

Initial commit                                                                                                                 

Changes to be committed:                                                                                                       
  (use "git rm --cached <file>..." to unstage)                                                                                 

        new file:   a                                                                                                          
        new file:   git/a                                                                                                      

[gittest] $ git add -f git/.git/                                                                                               
error: Invalid path 'git/.git/a'                                                                                               
error: unable to add git/.git/a to index                                                                                       
fatal: adding files failed                                                                                                     

@andsens
Copy link
Owner Author

andsens commented Apr 24, 2014

Balls. Maybe we should've tried that to begin with? Hehe.

I can't see a way around this unless we have a "special" name for custom .git folders. It's ugly but considering how the feature can be really useful to some people I'd be OK with it.

In #77 I propose adding a separation char to support linking files and folders depending on the system homeshick is running on. # seems like a decent choice to me, so how about using that here as well? .git# would be translated to .git.

Though maybe a doublehash would be better seeing how people might enumerate some foldernames like .folder #1, .folder #2 etc. (?)

@squaresurf
Copy link
Contributor

Yeah, should have tested that first. Oh well.

#77 is a really nice feature. I've been separating my os dependent files into separate castles. I like the idea that a blank os defaults to all. For example: .git#linux would symlink only on linux while .git# would symlink on all systems. It would probably be a good idea to add an escape character for it. We could allow home/.git\# and home/.git\\\# which would result in paths $HOME/.git# and $HOME/.git\#. The other thing we can do is to leave the hash alone if we don't find an os match. So that home/.folder #1 would become $HOME/.folder #1 since as far as I know there isn't an os named 1.

No matter how we go about this though there is a small chance that we will break some peoples configuration. Does homeshick use semantic versioning? If not, we could create a new issue to discuss it further. In essence we could add code that doesn't auto update homeshick if the version jump is a new major version (breaks backwards compatibility). Instead it would prompt the user, that there are major breaking changes and that they should read the change log before updating. After they've done so they will run homeshick update.

@ngaloppo
Copy link

I'm not exactly sure how #77 would break someone's existing config. Could you give an example?

@squaresurf
Copy link
Contributor

Upon more consideration I disagree with my idea I posted earlier:

The other thing we can do is to leave the hash alone if we don't find an os match. So that home/.folder #1 would become $HOME/.folder #1 since as far as I know there isn't an os named 1.

It is simply not intuitive that the # is sometimes removed and sometimes its now. Here are two problems it creates:

  • User Confusion: Why do some of my castles get the #* removed and others don't?
  • Support is added for a new os and potentially breaks castles where the user hasn't escaped the #newos.

@ngaloppo
Copy link

@squaresurf You had mentioned before that you currently use os-specific castles. How would that compare with this proposed feature?

@andsens
Copy link
Owner Author

andsens commented Apr 24, 2014

It would probably be a good idea to add an escape character for it.

I agree. The approach you outlined would be my choice as well.

The other thing we can do is to leave the hash alone if we don't find an os match.

Meh. I think that just makes things complicated when we decide to add more options after the hash (if for example we decide to support things other than the OS, e.g. the hostname). But let's discuss that in #77.

Does homeshick use semantic versioning? If not, we could create a new issue to discuss it further.

It doesn't yet, but I think the time is ripe for that. Even though I'd like to keep things simple, I can see that removing legacy stuff will be almost impossible to do without it.
Can you copy that last paragraph and create a new issue for that?

@andsens
Copy link
Owner Author

andsens commented Apr 24, 2014

Seeing how I don't didn't touch upon the issue of .git# directly, let's do that now :-)
.git# is the one that looks best to me, the only trouble I have is that it should fit together with the semantics of #77. So what does folder# mean? Is it like leaving a search field empty and getting all possible results, meaning "link" it on all systems? I think that's the best way of going about it.

.folder# is just the same as writing .folder, the # will be removed when linked and that's it.

@squaresurf
Copy link
Contributor

I agree 👍

andsens added a commit that referenced this issue May 16, 2014
This resolves #95 : Only symlink files that are tracked by git
@chibicode
Copy link

👍

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

No branches or pull requests

4 participants